Log orphaned attachments action for tests

[labkey-adam]

Rationale

AttachmentHelper.logOrphanedAttachments() gives the tests a way to proactively check for orphaned attachments

Related Pull Requests

• Log orphaned attachments action for tests platform#7556

[labkey-tchad]

I'd be happy to have this check run after every test.

[labkey-tchad]

The test runner always forces BasicTest to run first so this won't catch anything on TeamCity. I'd suggest adding this check to BaseWebDriverTest.doPostamble, around where we run the crawler (checkLinks) and check for CSP warnings (checkNewCspWarnings).
It should also throw an error if any orphaned attachments are found, otherwise we won't ever notice them.

[labkey-tchad]

Actually, I didn't notice that the logOrphanedAttachments action will make ERROR logs. Those will already cause the test to fail so there's no need to add the extra fail case from the orphaned attachment check itself.

[labkey-adam]

I can remove the check from BasicTest. The product still checks and logs errors for orphaned attachments before every container delete, so will doPostamble check anything that delete container won't?

[labkey-tchad]

Oh. Yeah, adding the check to doPostamble wouldn't accomplish much in that case. I thought the existing orphaned attachment check happened after container deletion and that we were adding an extra check for items that are cleaned up by container deletion but might be orphaned by other means.

[labkey-tchad]

Maybe add testOrphanedAttachments to DatabaseDiagnosticsTest instead. It's in a lot of suites and is always the final test of a suite.

[labkey-adam]

@labkey-tchad and anyone else who's interested, orphaned attachments are logged (and will fail the test):

1. Just before any container is deleted. This will detect the case where we delete an object with an attachment but fail to delete its attachments. This will also detect attachment parent types that fail to claim an attachment that belongs to it.
2. When DatabaseDiagnosticsTest runs. I'm not sure this adds much beyond #⁠1 above, but it doesn't hurt.
3. Just before compounds are truncated in the cleanup that happens before every test case in BioCompoundSMILESTest. This will detect regressions with ExpDataClassType's attachment detection.

We should consider adding calls to AttachmentHelper.logOrphanedAttachments() both before and after deleting objects with attachments. The before case tests the attachment parent type logic and the after case tests that the delete code deletes the associated attachments. We should also audit all objects that can have attachments, making sure we add them and delete them outside of container delete. Container delete often uses a different code path vs. single object delete.