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

Re-enable testing on PyPy for all platforms. #553

Merged
merged 2 commits into from Jun 18, 2022

Conversation

bwoodsend
Copy link
Collaborator

  • Renable CI/CD testing on PyPy for all platforms.
  • xfail test_decode_surrogate_characters() on Windows PyPy.

Addresses (but doesn't fix) #552.

PyPy's Windows unicode handling transpired to be significantly different
enough that a PyPy+Windows specific test failure slipped in (ultrajson#552).
@codecov-commenter
Copy link

codecov-commenter commented Jun 17, 2022

Codecov Report

Merging #553 (e500f3f) into main (b47c3a7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head e500f3f differs from pull request most recent head 404de1a. Consider uploading reports for the commit 404de1a to get more accurate results

@@           Coverage Diff           @@
##             main     #553   +/-   ##
=======================================
  Coverage   91.83%   91.83%           
=======================================
  Files           6        6           
  Lines        1824     1825    +1     
=======================================
+ Hits         1675     1676    +1     
  Misses        149      149           
Impacted Files Coverage Δ
tests/test_ujson.py 99.44% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b47c3a7...404de1a. Read the comment docs.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Three of the 15 parametrised cases pass and show as xpasses instead of xfails:

tests\test_ujson.py ........................sssssssssss.............................XXxxxxxxxxxxxxs.......................................................................................................................................................................................s..s
=========== 238 passed, [14](https://github.com/ultrajson/ultrajson/runs/6943256123?check_suite_focus=true#step:5:15) skipped, 12 xfailed, 2 xpassed in 7.00s ============

We could add a condition to only skip those that really do fail (and possibly add strict checking so xpasses fail), but we'll have a fix soon so this is fine. 👍

Comment on lines 17 to 18
- { python-version: "3.10", os: windows-latest }
- { python-version: "3.10", os: macos-latest }
- { python-version: "pypy-3.8", os: windows-latest }
- { python-version: "pypy-3.8", os: macos-latest }
Copy link
Member

Choose a reason for hiding this comment

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

PyPy tests are a bit slower than CPython, let's give them a bit of a headstart:

Suggested change
- { python-version: "3.10", os: windows-latest }
- { python-version: "3.10", os: macos-latest }
- { python-version: "pypy-3.8", os: windows-latest }
- { python-version: "pypy-3.8", os: macos-latest }
- { python-version: "pypy-3.8", os: windows-latest }
- { python-version: "pypy-3.8", os: macos-latest }
- { python-version: "3.10", os: windows-latest }
- { python-version: "3.10", os: macos-latest }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm fairly certain that the order in which you declare them is ignored. I've tried reording jobs before and it didn't make any difference. That was a while ago though so it might have changed.

Copy link
Member

Choose a reason for hiding this comment

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

Here's a comparison:

yesterday now
image image

@hugovk hugovk added changelog: skip Exclude PR from release draft testing Unit tests, linting, CI, etc. labels Jun 18, 2022
@bwoodsend bwoodsend force-pushed the pypy-ci branch 2 times, most recently from e500f3f to 404de1a Compare June 18, 2022 17:13
@bwoodsend
Copy link
Collaborator Author

Oohps, pushed that last commit to the wrong branch...

@hugovk
Copy link
Member

hugovk commented Jun 18, 2022

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: skip Exclude PR from release draft testing Unit tests, linting, CI, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants