Skip to content

Extra Windows Helpers #2585

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

Closed
labkey-martyp wants to merge 5 commits into from
Closed

Extra Windows Helpers #2585

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fd9572c
helper function
labkey-martyp Jul 20, 2025
d4bb988
Better win count
labkey-martyp Jul 28, 2025
97bc9a2
add openLinkInNewWindowOrThrow
labkey-martyp Jul 28, 2025
1804409
Merge remote-tracking branch 'origin/release25.7-SNAPSHOT' into 25.7_…
labkey-martyp Jul 30, 2025
6c4c660
CR feedback
labkey-martyp Jul 30, 2025
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
15 changes: 14 additions & 1 deletion src/org/labkey/test/WebDriverWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2872,8 +2872,21 @@ public void clickAndWait(Locator l)

public void openLinkInNewWindow(WebElement link)
{
int winCount = getDriver().getWindowHandles().size();
link.sendKeys(Keys.chord(WebDriverUtils.MODIFIER_KEY, Keys.ENTER));
switchToWindow(1);
switchToWindow(winCount);
waitForDocument();
}

public void openLinkInNewWindowOrThrow(WebElement link)
{
Comment on lines +2881 to +2882
Copy link
Member

@labkey-tchad labkey-tchad Jul 28, 2025

Choose a reason for hiding this comment

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

This method is redundant. switchToWindow will throw if the specified window index doesn't exist. That being said, I wouldn't hate if it threw a more informative error (currently, you'd just get an IndexOutOfBoundsException)

Copy link
Contributor Author

@labkey-martyp labkey-martyp Aug 1, 2025

Choose a reason for hiding this comment

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

yeah switchToWindow waits 10 secs before throwing that IndexOutOfBoundsException. Since we're using this as a test to discover anchor tags that are links, there will be a relative high number of failures. So openLinkInNewWindowOrThrow speeds up that check for this use case and throws a more explicit exception. If you think it's ok to refactor openLinkInNewWindow to more quickly check and throw then we could use it here without slowing down this test.

Copy link
Member

@labkey-tchad labkey-tchad Aug 1, 2025

Choose a reason for hiding this comment

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

That's still a pretty misleading method name. Maybe just add timeout parameters to openLinkInNewWindow and switchToWindow

Copy link
Contributor Author

@labkey-martyp labkey-martyp Aug 3, 2025

Choose a reason for hiding this comment

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

Ok, that would be a pretty big refactor, switchToWindow is used in a lot of places. Maybe I'm pushing too hard to use this general functionality for a specific use case it doesn't quite match. I'll close this PR and do a more focused function in ehrModules.

int winCount = getDriver().getWindowHandles().size();
link.sendKeys(Keys.chord(WebDriverUtils.MODIFIER_KEY, Keys.ENTER));
boolean winOpen = waitFor(() -> getDriver().getWindowHandles().size() > winCount, 1000);
if (!winOpen)
throw new IllegalStateException("Link did not open new window in tab.");

switchToWindow(winCount);
waitForDocument();
}

Expand Down