Fix browser.stopped property. Add test

[puc9]

Description

Code that polls for the browser process to stop without calling browser.stop() first will never detect if the browser has stopped.

One example is when running a full browser with UI and the user closes it manually. If a script checks browser.stopped it will always return false.

The reason is the _process.returncode is not updated automatically. You would need to call _process.poll() or _process.wait().

The fix is to replace checking _process.returncode with checking _process.poll().

I've also added a test that will fail without the fix but pass with the fix.

This exposes the fact that browser.stop() will fail if the browser connection is lost somehow. To fix that I made the Connection class be truthy about being closed and added a try - except block for the stop function.

Added tests for validation

Pre-merge Checklist

• I have described my change in the section above.
• I have ran the ./scripts/format.sh and ./scripts/lint.sh scripts. My code is properly formatted and has no linting errors.
• I have ran uv run pytest and ensured all tests pass.
• I have added my change to CHANGELOG.md under the [Unreleased] section.

[stephanlensky]

Thank you for the submission and for the added tests! This looks great, if you wouldn't mind just addressing the one comment about not overriding __bool__ I can get this merged.

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
zendriver/core/browser.py	66.66%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[stephanlensky]

Hm, it looks like one of the new tests might be failing on Windows, would you mind taking a look?

[puc9]

Hm, it looks like one of the new tests might be failing on Windows, would you mind taking a look?

This passes on my machine...
There might be something I don't know about pytest. The test failed because psutil raised TimeoutError which is expected and, I thought appropriately caught in the except block. Is that not the case?

[puc9]

Hm, it looks like one of the new tests might be failing on Windows, would you mind taking a look?

This passes on my machine... There might be something I don't know about pytest. The test failed because psutil raised TimeoutError which is expected and, I thought appropriately caught in the except block. Is that not the case?

Strike all that... psutil defines their own exception, TimeoutExpired 🤦 while I was catching the standard lib one, TimeoutError. And it passes on my machine because it is likely faster and doesn't reach the timeout even once.
I'll add a fix soon.

[stephanlensky]

Thanks for looking more. I was actually wondering, do you think we need psutil here? I'm not sure but maybe there is a simple way to check if a process is running with just the standard lib?

If not though it's np

[puc9]

I'm not sure, I looked for one that works on Windows as well and couldn't find it. I'll have another look for a few mins, I'll reply back when done.

[puc9]

I'm not sure, I looked for one that works on Windows as well and couldn't find it. I'll have another look for a few mins, I'll reply back when done.

OK, I remember my logic at the time now. We can't use the stdlib because, AFAICT, there is no way to query for a process state by ID.
If we would wait for the process using the internal browser._process field, then that would not expose the bug as the wait() call would update the browser._process.returncode and then the bug wouldn't manifest itself. Basically I was looking for a way to emulate a browser being closed from outside our program and have a deterministic way of checking when it is no longer running.
The new dependency is dev only so, although not great, I think it is unavoidable.