Skip to content
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.

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

On session close, don't close apps that weren't started by the WebDriver #31

Merged
merged 4 commits into from
May 2, 2024

Conversation

grokys
Copy link
Contributor

@grokys grokys commented May 2, 2024

When a session finishes or times out, don't close the app if the session was attached to an existing app using appTopLevelWindow or appTopLevelWindowTitleMatch.

This feels like intuitive behavior to me: the app was running before the session started so one can assume that the user is in control of the app lifetime.

Added a commit with failing tests, and a commit with a simple fix.

grokys added 2 commits May 2, 2024 15:15
When a session times out, don't close the app if the session was attached to an existing app using `appTopLevelWindow` or `appTopLevelWindowTitleMatch`.
Copy link
Collaborator

@aristotelos aristotelos left a comment

Choose a reason for hiding this comment

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

Looks great, added a few comments

src/FlaUI.WebDriver.UITests/SessionTests.cs Outdated Show resolved Hide resolved
src/FlaUI.WebDriver.UITests/SessionTests.cs Outdated Show resolved Hide resolved
src/FlaUI.WebDriver.UITests/SessionTests.cs Show resolved Hide resolved
@aristotelos
Copy link
Collaborator

Is this exposing the same behavior as other drivers, e.g. appium-windows-driver?

The specification is not completely clear about what is best, see https://www.w3.org/TR/webdriver2/#dfn-close-the-session

@grokys
Copy link
Contributor Author

grokys commented May 2, 2024

Is this exposing the same behavior as other drivers, e.g. appium-windows-driver?

Hmm, as far as I remember, WinAppDriver doesn't have a "close app on timeout" feature; I don't know if appium-windows-driver adds that. I can check.

Edit: I have the "timeout" part in my head because that's where I kept encountering this - I'd stop a test that I was debugging mid-run, the session would time out and the process I was debugging would exit. But it's not just on timeout that the app is being closed, it's when the session ends in general, so ignore the timeout part above ;)

@grokys
Copy link
Contributor Author

grokys commented May 2, 2024

Just checked WinAppDriver and it doesn't appear to stop a process started with appTopLevelWindow when the session ends normally.

@aristotelos aristotelos merged commit dac4fab into FlaUI:main May 2, 2024
3 checks passed
@grokys grokys deleted the fixes/dont-close-attached-apps branch May 2, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants