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 pytest extra args option #59

Merged
merged 4 commits into from
Dec 14, 2022
Merged

Conversation

joemarshall
Copy link
Collaborator

This adds an option to the reusable main action for extra args to pass to pytest. Without that it isn't possible to e.g. filter out non-pyodide test python files.

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, @joemarshall!

@@ -165,7 +169,8 @@ jobs:
--cov=pytest_pyodide \
--dist-dir=./pyodide-dist/ \
--runner=${{ inputs.runner }} \
--rt ${{ inputs.browser }}
--rt ${{ inputs.browser }} \
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
--rt ${{ inputs.browser }} \
--rt=${{ inputs.browser }} \

Without this, pytest-extra-args will be push to runtime list

Copy link
Member

Choose a reason for hiding this comment

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

Oh, okay this change breaks one test, which passes multiple browsers to inputs.browser. I think I added that to test whether --rt correctly handles multiple parameters. But I think we can remove that test from filtermatrix.py, regarding that users who use this action don't need that test.

Co-authored-by: Gyeongjae Choi <def6488@gmail.com>
bwoodsend pushed a commit to ultrajson/ultrajson that referenced this pull request Nov 3, 2022
Doing so allows for building on webassembly. Officially declareing support for WASM
however will have to wait until pyodide/pytest-pyodide#59 is
merged to facilitate testing WASM compatibility.
@rth
Copy link
Member

rth commented Dec 10, 2022

What's the status of this PR? Do we still want it or is now handled by pyodide-actions repo?

@ryanking13
Copy link
Member

ryanking13 commented Dec 12, 2022

I'll open a PR that fixes the failing test suite, then we can merge this. This isn't related to pyodide-actions I think.

@ryanking13
Copy link
Member

Thanks @joemarshall!

@ryanking13 ryanking13 merged commit bd0221a into pyodide:main Dec 14, 2022
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.

3 participants