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

Pre-empted driver quitting with a .close #4

Merged
merged 3 commits into from Aug 23, 2018
Merged

Pre-empted driver quitting with a .close #4

merged 3 commits into from Aug 23, 2018

Conversation

JoshuaKGoldberg
Copy link
Contributor

See mozilla/geckodriver#1151. It looks like at least Firefox will annoyingly remain open if a page has certain kinds of modal dialogs.
Although it's a teeny bit messy, I don't think it can hurt to really close the browser.

See mozilla/geckodriver#1151. It looks like at least Firefox will annoyingly remain open if a page has certain kinds of modal dialogs.
Although it's a teeny bit messy, I don't think it can hurt to _really_ close the browser.
@alexeyraspopov
Copy link
Owner

I hope this also fixes an issue with Safari that I've observed a while ago. Let me check the changes locally, before merging this in.

Thanks for doing this!

@JoshuaKGoldberg
Copy link
Contributor Author

Ping @alexeyraspopov - any update? 😊

@JoshuaKGoldberg
Copy link
Contributor Author

Added another fix here: sometimes, if there's an error in setup, this.driver is undefined. That causes teardown to fail on the .close().

Solution: check if (this.driver).

@JoshuaKGoldberg
Copy link
Contributor Author

JoshuaKGoldberg commented Aug 23, 2018

....and this opens #8. 😂

Edited PR to address it preemptively.

@alexeyraspopov alexeyraspopov merged commit 129c376 into alexeyraspopov:master Aug 23, 2018
@alexeyraspopov
Copy link
Owner

Sorry for this huge delay. I didn't have enough time to continue this project. I'm really sorry.

@JoshuaKGoldberg JoshuaKGoldberg deleted the driver-close-quit-fix branch August 23, 2018 17:48
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.

None yet

2 participants