Skip to content

Convert test runner to use pypiserver package as standalone process #5284

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

Merged
merged 39 commits into from
Sep 5, 2022

Conversation

matteius
Copy link
Member

@matteius matteius commented Aug 23, 2022

  • Converts the pipenv test runner to use pypiserver.
  • Invokes pypiserver correctly for all supported OS types
  • Uses waitress for Windows only because it reduced the run time of windows test runner by 10 minutes from the default wsgi bottle.
  • Does not use waitress for Ubuntu because it slowed it down significantly, nor for mac OS which showed no difference using it.
  • Refactor test naming and cleanup conftest file to be more certain about temp directory cleanup
  • Tableflip on the dep_to_pip style tests and realize that there is still some cross tests contamination going on that sometimes confused requirementslib into adding a specifier for requests without a specifier -- I ultimately concluded that I don't have time currently to figure out how to prevent this other than to convert those requests and requests with extras pip_to_deps tests to use uvicorn as its test subject instead, since it too has extras and we don't use it in other tests that pull in the vcs setup.py metadata artifacts. We can see on the latest main build this failed again too, so we basically have been getting lucky and this change to uvicorn at least gurantees the build won't be flaky anymore for this particular reason: https://github.com/pypa/pipenv/runs/8179118655?check_suite_focus=true
  • Updated dev documentation and the scripts for invoking pytest to startup the pypiserver ahead of time.
  • I did some speed analysis and I feel this is on par with our current test runner, but is way better because we eliminate the fixtures that invoke several threads of an old unsupported pypiserver and instead the tests are CPU lighter now when they run even if they take a similar amount of time.
  • This really opens the door to better debugging because it becomes easy to access the same private pypi server that the tests are referencing and check its behavior and the available packages. The options we can pass to this pypiserver are way more expansive, and likely to improve overtime as that project has a new maintainer that appears interested in accepting contributions. For example, right now there is no way to specify thread count to the bottle wsgi server, and so for waitress it defaults to 4, but the test runner has 2 cores so this is overkill -- we can probably get in the future an ability to specify this value to experiment with runtimes.
  • We are closer than ever with this PR to being able to support private pypi tests that involve credentials 🎉
  • Probably other benefits to doing this.

Deferred for Future

Artifacts and Stats

image

image

First run where this PR refactor was really successful:

image

Second run where I pushed doc/news changes and let it re-run:

image

###Latest main branch run (for possible comparison)
image

The checklist

  • A news fragment in the news/ directory to describe this fix with the extension .bugfix.rst, .feature.rst, .behavior.rst, .doc.rst. .vendor.rst. or .trivial.rst (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

…this does not work due to AttributeError: 'HTMLParser' object has no attribute 'unescape'
@matteius matteius changed the title Test pypiserver Convert test runner to use pypiserver package as standalone process Sep 5, 2022
@matteius matteius requested a review from oz123 September 5, 2022 00:46
@matteius matteius added PR: awaiting-review The PR related to this issue is awaiting review by a maintainer. Category: Tests Relates to tests. labels Sep 5, 2022
@pytest.mark.skip_windows
@pytest.mark.skipif(sys.version_info >= (3, 9), reason="old setuptools doesn't work")
@pytest.mark.needs_internet
def test_outdated_setuptools_with_pep517_legacy_build_meta_is_updated(PipenvInstance):
Copy link
Member Author

Choose a reason for hiding this comment

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

These tests are the black magic of the devil and old setuptools -- how they ever passed I have no clue and vote we should drop them now. Just read the comments, it feels like a very old problem it was checking for, and with time we would drop these tests anyway.

@oz123
Copy link
Contributor

oz123 commented Sep 5, 2022

I believe, though I haven't tested it, that adding gunicorn on Ubuntu and Mac might improve the speed of the tests.
WSGIRef is darn slow, especially when running in parallel

@matteius
Copy link
Member Author

matteius commented Sep 5, 2022

I believe, though I haven't tested it, that adding gunicorn on Ubuntu and Mac might improve the speed of the tests. WSGIRef is darn slow, especially when running in parallel

It might have helped ubuntu but I think it hurt Mac OS run time so I am going to change gunicorn to just be installed for linux.
image

@matteius matteius merged commit 9848862 into main Sep 5, 2022
@matteius matteius deleted the test-pypiserver branch September 5, 2022 14:19
yeisonvargasf pushed a commit to yeisonvargasf/pipenv that referenced this pull request Nov 19, 2022
…ypa#5284)

* Check point progress on moving tests to pypiserver.

* Allow other indexes to mimic the pypi json API for package hashes.

* Fix these tests that run on lower python versions only.

* Try adding the pypiserver to the github actions to only run once.

* Expand the scope of fixtures for pypiserver.

* try to accomedate microsoft runner.

* Windows networking troubles.

* Remove running as a background job.

* Try to condtionally invoke different start pypi-server commands

* Try to condtionally invoke different start pypi-server commands

* Try to condtionally invoke different start pypi-server commands

* Try to condtionally invoke different start pypi-server commands

* Try to condtionally invoke different start pypi-server commands

* Try to condtionally invoke different start pypi-server commands

* Try to condtionally invoke different start pypi-server commands

* Try to introduce pypi as the root index because setuptools-scm is not in our pypi artifacts.

* see if the windows tests run faster (and the other tests) by supplying waitress.

* Only use waitress on windows because the others are fast on the default.  Fix requests pollution.

* Supply a suitable Pipfile instead for these two failing tests.

* More requests resolver cross test contamination cleanup.

* remove problematic tests because even on my version of python 3.8.12 this does not work due to AttributeError: 'HTMLParser' object has no attribute 'unescape'

* fix mirror install test.

* Fix Pipfile.

* Fix Pipfile for real

* Fix tests

* Cleanup test naming and more test enhancements.

* Save this optimization for a subsequent PR.

* Cleanup the TemporaryDirectory between tests.

* resolve merge conflict.

* Cleanup path initalization -- it hsould always be a TemporaryDirectory for tests that gets cleanedup.

* Cleanup path initalization -- it hsould always be a TemporaryDirectory for tests that gets cleanedup.

* tableflip on those requests tests that read the setup metadata in reqlib from other tests.

* Update developer documentation for running tests.

* add news fragment.

* Try gunicorn perfoormance for linux/mac

* Only use gunicorn on linux based on the results of last run.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Tests Relates to tests. PR: awaiting-review The PR related to this issue is awaiting review by a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants