Skip to content

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

Merged
hugovk merged 2 commits intoultrajson:mainfrom
bwoodsend:pypy-ci
Jun 18, 2022
Merged

Re-enable testing on PyPy for all platforms.#553
hugovk merged 2 commits intoultrajson:mainfrom
bwoodsend:pypy-ci

Conversation

@bwoodsend
Copy link
Copy Markdown
Collaborator

@bwoodsend bwoodsend commented Jun 17, 2022

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

Addresses (but doesn't fix) #552.

@bwoodsend
Renable CI/CD testing on PyPy for all platforms.
fb6251e 
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
Copy Markdown

codecov-commenter commented Jun 17, 2022 •
edited
Loading

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.

hugovk
hugovk reviewed Jun 18, 2022
View reviewed changes
Copy link
Copy Markdown
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 thread .github/workflows/test.yml
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
Copy Markdown
Member

@hugovk hugovk Jun 18, 2022

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
Copy Markdown
Collaborator Author

@bwoodsend bwoodsend Jun 18, 2022

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
Copy Markdown
Member

@hugovk hugovk Jun 18, 2022

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
@hugovk hugovk mentioned this pull request Jun 18, 2022
Merged
@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
Copy Markdown
Collaborator Author

bwoodsend commented Jun 18, 2022

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

@hugovk
Copy link
Copy Markdown
Member

hugovk commented Jun 18, 2022

Thank you!

@hugovk hugovk merged commit 4b5cccc into ultrajson:main Jun 18, 2022
@bwoodsend bwoodsend deleted the pypy-ci branch June 18, 2022 17:48
@sync-by-unito sync-by-unito Bot mentioned this pull request Jul 11, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk left review comments

Assignees

No one assigned

Labels

changelog: skip Exclude PR from release draft testing Unit tests, linting, CI, etc.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bwoodsend @codecov-commenter @hugovk