-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[10171] Fix coverage run random failures on PYPY due to SQLite error. #1581
Conversation
My plan is to have this is trunk for a while... 1 month. If we no longer observer pypy coverage errors. we can consider it fixed. report upstream and use the released version with the fix, when ready, Thanks! |
coverage ~= 5.5 | ||
# To be pinned to a minor version once PYPY `coverage run` error is fixed. | ||
# See: https://twistedmatrix.com/trac/ticket/10171 | ||
coverage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we drop the tox.ini
change if we do this here?
coverage | |
coverage @ https://github.com/nedbat/coveragepy/archive/refs/heads/nedbat/another-1010.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, actually I suppose...
coverage | |
coverage ~= 5.5 @ https://github.com/nedbat/coveragepy/archive/refs/heads/nedbat/another-1010.zip |
Or describe the version it presently lists, though this is going a bit far probably. The first suggestion in the previous comment is likely sufficient and useful since it isolates the change to only needing to be undone in one place.
https://github.com/nedbat/coveragepy/blob/nedbat/another-1010/coverage/version.py#L8
version_info = (5, 5, 1, "alpha", 0)
coverage | |
coverage >= 5.5.1, < 6 @ https://github.com/nedbat/coveragepy/archive/refs/heads/nedbat/another-1010.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that we can drop it.
I tried your last suggestion and I got
pkg_resources.extern.packaging.requirements.InvalidRequirement: Parse error at "'@ https:'": Expected stringEnd
I think that a bit of duplication is ok and there is not much gain by "doing it right(tm)"
I think that this is no longer supported...and it required an extra dependency_links
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@
is a new syntax but apparently it doesn't work there... I'll have to go learn. We can move on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(this is not in setup.cfg
, just an example in a place without dependency_links
.)
$ venv/bin/pip install 'coverage @ https://github.com/nedbat/coveragepy/archive/refs/heads/nedbat/another-1010.zip'
Collecting coverage@ https://github.com/nedbat/coveragepy/archive/refs/heads/nedbat/another-1010.zip
Downloading https://github.com/nedbat/coveragepy/archive/refs/heads/nedbat/another-1010.zip
/ 1.0 MB 1.5 MB/s
Building wheels for collected packages: coverage
Building wheel for coverage (setup.py) ... done
Created wheel for coverage: filename=coverage-5.5.1a0-cp39-cp39-linux_x86_64.whl size=243403 sha256=de379aeb239ebb6321429001d7085a07748ffb5277bd9692e50c72bf692b885a
Stored in directory: /home/altendky/.cache/pip/wheels/3e/b1/2f/368d0a88f96d28bf2f5f1e74203b32ac575b293fe0d40f4bcd
Successfully built coverage
Installing collected packages: coverage
Successfully installed coverage-5.5.1a0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mm, looks like maybe just the version spec is the issue. Either version or @ but not both. Which seems reasonable.
Thanks for the review. I hope a new coverage release will be made soon and we can then remove this. |
Scope and purpose
Coverage runs on PYPY fails
https://github.com/twisted/twisted/pull/1577/checks?check_run_id=2274177993#step:9:751
Experimental fix discussed here nedbat/coveragepy#1010
This is the Twisted integration branch.
Contributor Checklist:
tox -e lint
to format my patch to meet the Twisted Coding Standard#
character).review
to the keywords field in Trac, and putting a link to this PR in the comment; it shows up in https://twisted.reviews/ now.The first line is automatically generated by GitHub based on PR ID and branch name.
The other lines generated by GitHub should be replaced.