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

Add packse scenarios as pip tests #12580

Closed

Conversation

notatallshaw
Copy link
Member

@notatallshaw notatallshaw commented Mar 17, 2024

Fixes #12526

Current strategy:

  1. Run packse fetch which git pulls the packse scenarios
  2. Run packse serve which builds and serves the scenarios on a local index
  3. Run each scenario via test_packse_scenario as a dynamically generated parameter from the information given in packse inspect

I do not have a lot of experience with test frameworks, so I'm not at all claiming this is the best approach, it's just what I got working, happy to significantly change it.

There is still more work to do discussing issues on packse side and identifying if currently failing scenarios are bugs on pip side. Once EXPECTED_TO_FAIL is much smaller I will unmark as draft.

Comment on lines +85 to +93
@pytest.fixture(scope="session", autouse=True)
def start_packse_server() -> Generator[None, None, None]:
"""Starts the packse server before tests run and ensures it's terminated after."""
proc = subprocess.Popen(
["packse", "serve", "--host", "127.0.0.1", "--port", "3141"]
)
time.sleep(1)
yield
proc.terminate()
Copy link

@zanieb zanieb Mar 17, 2024

Choose a reason for hiding this comment

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

I would recommend using the static index packse generates as mentioned in astral-sh/packse#165 (comment) instead — the serve command is intended for development purposes.

Copy link
Member Author

Choose a reason for hiding this comment

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

My concern with that, and maybe it's not justified, is that tests could fail because the scenarios from packse fetch & packse inspect would not align perfectly with the scenarios on the index.

Copy link

Choose a reason for hiding this comment

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

fetch defaults to pulling the current packse version tag which should always match the corresponding index for that tag. You just need to do packse inspect --no-hash since the index is versioned we don't need to include a hash to uniquely identify scenarios.

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 had a long throught about this and my main concern is that if pip wants to add it's own scenarios, or deviate away from packse scenarios (e.g. because they seemingly don't follow the spec astral-sh/packse#161) then using the astra index won't work.

However, for now, I will attempt to implement using the index, and that problem can be worried about in the future.

@notatallshaw
Copy link
Member Author

Some unexpected failures on Mac, but passed on Windows and Linux, probably won't have time to look this weekend but I'm sure I'll be able to figure them out.

@notatallshaw
Copy link
Member Author

notatallshaw commented Jun 27, 2024

I am going to close this for now. I think packse is a great tool, and integrating it into pip's testing would be very useful.

But I don't think pip can use all of the scenarios packse provides out of the box or the ones astral provides on it's index. It will therefore require maintenance of either an "allow" list of packse scenarios, or maintaining a separate collection of scenarios, this would therefore introduce a hidden maintenance cost that the PR does not make clear.

Some of the issues are (in no particular order):

  • Packse includes scenarios that apply to features uv has that pip does not have
  • Packse includes scenarios that test uv design choices that do not comply with the spec
  • pypiserver is missing some features and therefore some scenarios can not be constructed locally

IMO, to achieve "testable standards" (as Charlie mentioned) I think there needs to be a delineation of scenarios, either by a flag or living in a separate repo, of those that are derived from standards and ones that are testing non-standard installer features or design choices. And, somehow, more members of the interested parties (those writing and discussing standards) need to be involved. But I think such a discussion needs to move to https://discuss.python.org/c/packaging/14.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add packse (Python packaging scenarios) to Pip's test suite
2 participants