test: add more regression tests for self-referential extras - #14161
Conversation
|
Please clarify your description of the tests here. In your own words, I don't want to have a discussion with an LLM. What do you mean by tests being "Highly Unlikely to Fail" or "Could Potentially Fail". Tests are intended to never fail - any test that "might" fail is flaky, and we don't want flaky tests. If what you mean is that the tests check more or less likely scenarios, that's a very different matter (and I'd question why you thing one scenario is more likely than another). Sorry for nitpicking here, but I have no patience for dealing with LLM verbosity, and I'd like a human explanation of what you're hoping to achieve here. As I said on the issue, brevity is a virtue. |
Sorry about the confusing titles, I meant that case 1-7 described above should pass between 20.3 and 21.1.3; and 8-9 should only pass with 21.2+, I subdivided the first 7 cases into 2 groups based of how I felt like, but that may not make sense. The test case with PEP 685 normalization would start passing with 24.1+ so I have reordered to have it as the 9th case in PR description. I've also updated the PR description and merged 7-8 into one test function parameterized with two scenarios:
Then a new package version with an @pfmoore I've updated the PR description with how I'd describe each case, again, really sorry for the initial descriptions with too much LLM-ness inside. |
ichard26
left a comment
There was a problem hiding this comment.
I read through the tests. They're great. Thank you for being comprehensive.
However, as noted earlier, please be mindful of your AI/LLM usage going forward. It is particularly annoying for us to read LLM-generated text. It simply shifts the burden of trying to read and understand what is being said from the author to the reader, aka us.
I'll be mindful in future communications, again, sorry for the laziness in writing the initial PR description. |
What does this PR do?
As discussed in #14160, the current regression test introduced by #14157 was too naive, even old versions of pip (
v20.3 - v21.1.3) were able to pass.This PR added 9 more test cases to cover expected behaviour and prevent regression (local test results for v20.2 to latest & noxfile used are provided in #14160).
Closes #14160
Should pass with 20.3+
pkg[a]with an unrelated third-party dep in the same extra.pkg[a]is already installed, installingpkg[all]with the same version adds only the missing deps.pkg[missing]) must raise a warning as usual.dep==1to requiringdep==2) must bumpdepto version 2.adepends on extraband vice versa) must resolve normally (pip install pkg[a]installs dependencies from both extras).Should pass with 21.2+
7 & 8. upgrade-different-extras (two scenarios: had-no-extra or had-different-extras) — with an older pkg already installed (with no extras defined or with different extras defined and installed), upgrading to a new version with an
allextra should: 1) pull new deps and 2) downgrade an existing dep.Should pass with 24.1+
"all": ["pkg[x-y]"]to install extra defined asx_yinpyproject.toml.Corresponding test cases in
tests/functional/test_install_extras.py:nestedtest_install_self_referential_extras_nestedexternaltest_install_self_referential_extras_with_external_deppartialtest_install_self_referential_extras_after_partial_installunknowntest_install_self_referential_extras_unknown_nestednormalizetest_install_self_referential_extras_name_normalizationupgrade-deptest_install_self_referential_extras_upgrade_changes_dep_versioncirculartest_install_self_referential_extras_circularupgrade-different-extrastest_install_self_referential_extras_upgrade_different_extrasPR Checklist:
Disclosure:
Assisted-by: Cursor
It was used to write the nox file for testing the full matrix, and I have discussed the design of these test cases with it before implementation.