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

Add Pyodide handles #49

Merged
merged 8 commits into from
Oct 18, 2022
Merged

Add Pyodide handles #49

merged 8 commits into from
Oct 18, 2022

Conversation

hoodmane
Copy link
Member

@ryanking13 ran into the problem in pyodide/pyodide#3184 (comment) that it's inconvenient to send results from one @run_in_pyodide invocation to another. This adds a mechanism to do so. A SeleniumHandle (perhaps can be named better) can be returned to the host and when passed back into another @run_in_pyodide function one can use it to access the object.

This should allow us to make fixtures specifically for use with @run_in_pyodide functions that return one or more SeleniumHandles.

@ryanking13
Copy link
Member

Wow, thanks @hoodmane! I didn't look at the details yet but it looks very cool. I'll review this during weekends, or if it is not available, next Monday.

@ryanking13
Copy link
Member

ryanking13 commented Oct 14, 2022

@koenvo might want to review this too, as it looks like #46 has some code conflict with this.

@koenvo
Copy link
Contributor

koenvo commented Oct 14, 2022

I can change my PR when this one is merged. The provision_id is not used in my PR. Shouldn't conflict.

Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

Thanks! It looks quite interesting, and overall LGTM. Though I'll let @ryanking13 do a second review.

Could you please add a changelog entry and an example to the readme on how to use this?

Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

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

Thanks a lot @hoodmane! I have some minor comments, otherwise looks good to me.

pytest_pyodide/decorator.py Show resolved Hide resolved
pytest_pyodide/decorator.py Outdated Show resolved Hide resolved
pytest_pyodide/_decorator_in_pyodide.py Show resolved Hide resolved
@ryanking13
Copy link
Member

Thanks again! Please update the changelog and add some examples in README if possible as Roman mentioned, then feel free to merge. I think we can release this with the tag 0.23.0.

@hoodmane hoodmane changed the title Add selenium handles Add Pyodide handles Oct 18, 2022
@hoodmane hoodmane merged commit 4d64178 into pyodide:main Oct 18, 2022
@hoodmane hoodmane deleted the selenium-handle branch October 18, 2022 04:06
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.

4 participants