Skip to content

Log orphaned attachments action for tests #2940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
labkey-adam wants to merge 1 commit into
base: develop
Choose a base branch
from
+38 −1
Open

Log orphaned attachments action for tests #2940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/org/labkey/test/tests/BasicTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.labkey.test.categories.Hosting;
import org.labkey.test.categories.Smoke;
import org.labkey.test.pages.core.admin.ShowAdminPage;
import org.labkey.test.util.AttachmentHelper;
import org.labkey.test.util.Order;

import java.util.List;
Expand Down Expand Up @@ -62,6 +63,14 @@ public void testScripts()
assertTextNotPresent("WARNING:");
}

@Test
public void testOrphanedAttachments()
{
int orphanCount = AttachmentHelper.logOrphanedAttachments();
if (orphanCount > 0)
log("Orphaned attachments: " + orphanCount);
}

Comment on lines +66 to +73
Copy link
Copy Markdown
Member

@labkey-tchad labkey-tchad Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

@labkey-tchad labkey-tchad Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@labkey-adam labkey-adam Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member

@labkey-tchad labkey-tchad Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Test
public void testStartupLogging()
{
Expand Down
29 changes: 29 additions & 0 deletions src/org/labkey/test/util/AttachmentHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package org.labkey.test.util;

import org.labkey.remoteapi.CommandException;
import org.labkey.remoteapi.CommandResponse;
import org.labkey.remoteapi.SimpleGetCommand;
import org.labkey.test.WebTestHelper;

import java.io.IOException;

public class AttachmentHelper
{
/**
* Tells LabKey to log the first 20 orphaned attachments it detects at the ERROR level. Returns the total number of
* orphaned attachments detected. Must be a root admin (site or app) to call this.
*/
public static int logOrphanedAttachments()
{
SimpleGetCommand command = new SimpleGetCommand("admin", "logOrphanedAttachments");
try
{
CommandResponse response = command.execute(WebTestHelper.getRemoteApiConnection(), "/");
return response.getProperty("count");
}
catch (IOException | CommandException e)
{
throw new RuntimeException(e);
}
}
}
1 change: 0 additions & 1 deletion src/org/labkey/test/util/Crawler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@ protected List<ControllerActionId> getDefaultExcludedActions()
new ControllerActionId("login", "setPassword"),
new ControllerActionId("ms2", "showList"),
new ControllerActionId("ms2", "showParamsFile"),
new ControllerActionId("nlp", "runPipeline"),
new ControllerActionId("pipeline-analysis", "analyze"), // Doesn't navigate
new ControllerActionId("project", "togglePageAdminMode"),
// Tested directly in XTandemTest
Expand Down
Loading