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

[10171] Fix coverage run random failures on PYPY due to SQLite error. #1581

Merged
merged 2 commits into from
Apr 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion setup.cfg
Expand Up @@ -67,7 +67,9 @@ dev =
# TODO: support python-subunit in py3.10 https://twistedmatrix.com/trac/ticket/10115
python-subunit ~= 1.4; python_version < "3.10"
twistedchecker ~= 0.7
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
Copy link
Member

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?

Suggested change
coverage
coverage @ https://github.com/nedbat/coveragepy/archive/refs/heads/nedbat/another-1010.zip

Copy link
Member

Choose a reason for hiding this comment

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

Hum, actually I suppose...

Suggested change
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)
Suggested change
coverage
coverage >= 5.5.1, < 6 @ https://github.com/nedbat/coveragepy/archive/refs/heads/nedbat/another-1010.zip

Copy link
Member Author

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

https://setuptools.readthedocs.io/en/latest/userguide/dependency_management.html?highlight=dependency_links#dependencies-that-aren-t-in-pypi

Copy link
Member

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.

https://www.python.org/dev/peps/pep-0508/#examples

Copy link
Member

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

Copy link
Member

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.


tls =
pyopenssl >= 16.0.0
Expand Down
Empty file.
5 changes: 5 additions & 0 deletions tox.ini
Expand Up @@ -56,6 +56,11 @@ extras =
;; dependencies that are not specified as extras
deps =
lint: pre-commit
# For now we use a dev version of coverage with a fix for `coverage run`
# on PYPY.
# This should be moved
# See: https://twistedmatrix.com/trac/ticket/10171
withcov: https://github.com/nedbat/coveragepy/archive/refs/heads/nedbat/another-1010.zip

; All environment variables are passed.
passenv = *
Expand Down