Skip uninstallable tags in QPY backwards compatibility tests #13202
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a tag has been made, but the package has not yet landed on PyPI, the QPY job fails in the environment-building step. This is not actually a failure of the QPY backwards-compatibility guarantees, and it isn't the job of the QPY tests to detect a bad tag anyway.
jakelishman
added
type: qa
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 11013854521Details
💛 - Coveralls |
mtreinish
reviewed
Sep 23, 2024
There was a problem hiding this comment.
I'm a bit concerned about incorrectly skipping something we shouldn't and that masking our coverage for a version. I feel like we need to be a bit more intentional about when we're skipping as it should only be if we have yet to upload the package to pypi not for any other installation failure. The other changes here look fine to me though.
jakelishman
force-pushed
the
skip-uninstallable-qpy
branch
3 times, most recently
from
fe4de79
to
ca25cb1
Compare
The logic we actually want for versions to test is "find the versions that _should_ be installable", rather than "try it and see"; the latter is susceptible to silently suppressing errors. This new form now queries PyPI to find what versions of Qiskit are available in binary distributions for this platform, and filters based on that.
jakelishman
force-pushed
the
skip-uninstallable-qpy
branch
from
4070f76
to
394b6c1
Compare
mtreinish
reviewed
Sep 24, 2024
There was a problem hiding this comment.
Overall this looks fine to me. I personally find the new variable naming for the venvs and python in the bash scripts a bit harder to parse, but that's probably just how I think about working with this things is a bit more explicit in file paths. I only left one inline question about a change in behavior around rc releases. I'm fine if that's the direction you want to move here, but since it is a change I wanted to make sure it was intentional.
|
|
||
|
|
||
| def main(): | ||
| """main""" |
There was a problem hiding this comment.
passive aggressive suppression of the pylint warning haha
|
For the bash scripts venv renaming: the main problem for me locally was that they were all making a lot of strong assumptions about the working directory they were executed from, and one of them was making assumptions about the existence of a Python venv at a particular relative path, which meant I had to keep recreating the entire CI setup in order to test a single script. I generally agree the changes make it not as pleasant to read, but they decouple the scripts from particular working directories, and let me pass more information explicitly into each script, so it was easier to run things locally. |
mtreinish
approved these changes
Sep 24, 2024
|
@Mergifyio backport stable/1.2 stable/0.46 |
✅ Backports have been created
|
mergify bot
pushed a commit
that referenced
this pull request
Sep 24, 2024
* Skip uninstallable tags in QPY backwards compatibility tests When a tag has been made, but the package has not yet landed on PyPI, the QPY job fails in the environment-building step. This is not actually a failure of the QPY backwards-compatibility guarantees, and it isn't the job of the QPY tests to detect a bad tag anyway. * Query PyPI to find versions to test The logic we actually want for versions to test is "find the versions that _should_ be installable", rather than "try it and see"; the latter is susceptible to silently suppressing errors. This new form now queries PyPI to find what versions of Qiskit are available in binary distributions for this platform, and filters based on that. * Include release candidates in testing (cherry picked from commit 1344cdd) # Conflicts: # test/qpy_compat/process_version.sh # test/qpy_compat/run_tests.sh
mergify bot
pushed a commit
that referenced
this pull request
Sep 24, 2024
* Skip uninstallable tags in QPY backwards compatibility tests When a tag has been made, but the package has not yet landed on PyPI, the QPY job fails in the environment-building step. This is not actually a failure of the QPY backwards-compatibility guarantees, and it isn't the job of the QPY tests to detect a bad tag anyway. * Query PyPI to find versions to test The logic we actually want for versions to test is "find the versions that _should_ be installable", rather than "try it and see"; the latter is susceptible to silently suppressing errors. This new form now queries PyPI to find what versions of Qiskit are available in binary distributions for this platform, and filters based on that. * Include release candidates in testing (cherry picked from commit 1344cdd) # Conflicts: # test/qpy_compat/process_version.sh # test/qpy_compat/run_tests.sh
eliarbel
pushed a commit
to eliarbel/qiskit
that referenced
this pull request
Sep 24, 2024
…skit#13202) * Skip uninstallable tags in QPY backwards compatibility tests When a tag has been made, but the package has not yet landed on PyPI, the QPY job fails in the environment-building step. This is not actually a failure of the QPY backwards-compatibility guarantees, and it isn't the job of the QPY tests to detect a bad tag anyway. * Query PyPI to find versions to test The logic we actually want for versions to test is "find the versions that _should_ be installable", rather than "try it and see"; the latter is susceptible to silently suppressing errors. This new form now queries PyPI to find what versions of Qiskit are available in binary distributions for this platform, and filters based on that. * Include release candidates in testing (cherry picked from commit 1344cdd)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 25, 2024
…#13202) (#13217) * Skip uninstallable tags in QPY backwards compatibility tests (#13202) * Skip uninstallable tags in QPY backwards compatibility tests When a tag has been made, but the package has not yet landed on PyPI, the QPY job fails in the environment-building step. This is not actually a failure of the QPY backwards-compatibility guarantees, and it isn't the job of the QPY tests to detect a bad tag anyway. * Query PyPI to find versions to test The logic we actually want for versions to test is "find the versions that _should_ be installable", rather than "try it and see"; the latter is susceptible to silently suppressing errors. This new form now queries PyPI to find what versions of Qiskit are available in binary distributions for this platform, and filters based on that. * Include release candidates in testing (cherry picked from commit 1344cdd) # Conflicts: # test/qpy_compat/process_version.sh # test/qpy_compat/run_tests.sh * Resolve conflicts * Add qpy_teest_constraints.txt --------- Co-authored-by: Jake Lishman <[email protected]> Co-authored-by: Eli Arbel <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 25, 2024
…#13202) (#13216) * Skip uninstallable tags in QPY backwards compatibility tests (#13202) * Skip uninstallable tags in QPY backwards compatibility tests When a tag has been made, but the package has not yet landed on PyPI, the QPY job fails in the environment-building step. This is not actually a failure of the QPY backwards-compatibility guarantees, and it isn't the job of the QPY tests to detect a bad tag anyway. * Query PyPI to find versions to test The logic we actually want for versions to test is "find the versions that _should_ be installable", rather than "try it and see"; the latter is susceptible to silently suppressing errors. This new form now queries PyPI to find what versions of Qiskit are available in binary distributions for this platform, and filters based on that. * Include release candidates in testing (cherry picked from commit 1344cdd) # Conflicts: # test/qpy_compat/process_version.sh # test/qpy_compat/run_tests.sh * Resolve conflicts and add qpy_test_constraints.txt --------- Co-authored-by: Jake Lishman <[email protected]> Co-authored-by: Eli Arbel <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
When a tag has been made, but the package has not yet landed on PyPI, the QPY job fails in the environment-building step. This is not actually a failure of the QPY backwards-compatibility guarantees, and it isn't the job of the QPY tests to detect a bad tag anyway.
Details and comments
Since this commit is an acknowledgement that not every tag might be a valid version, and my editor was complaining about the safety of the various script calls, I made the variable expansion safe against word splitting, just in case.