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

Share script fixtures across tests #7276

Merged
merged 6 commits into from
Nov 2, 2019

Conversation

chrahunt
Copy link
Member

@chrahunt chrahunt commented Oct 30, 2019

We now have a script_factory pytest fixture that we can use to create reusable script objects. These can be used across multiple tests.

This helps because each usage of the plain script fixture requires:

  1. copying the whole template virtual environment
  2. (usually) copying test data (tests.lib.TestData via the data fixture)
  3. some setup/installation that is usually common across multiple tests

I just picked two example functional test files to start - I think there are a lot more opportunities to improve our test speed here.

Makes progress on #4497.

@chrahunt chrahunt added C: tests Testing and related things skip news Does not need a NEWS file entry (eg: trivial changes) labels Oct 30, 2019
@chrahunt chrahunt changed the title Shared test scripts Share script fixtures across tests Oct 30, 2019
@chrahunt chrahunt force-pushed the refactor/shared-test-scripts branch from b2376b4 to 191df15 Compare October 30, 2019 05:58
@chrahunt chrahunt force-pushed the refactor/shared-test-scripts branch from 191df15 to 0c4625b Compare October 30, 2019 06:18
@chrahunt chrahunt marked this pull request as ready for review October 30, 2019 11:54
@xavfernandez
Copy link
Member

Nice idea 👍
We might want to add a readonly attribute to those shared script to prevent pip install/uninstall calls ?

@chrahunt
Copy link
Member Author

chrahunt commented Nov 1, 2019

I expect the usage of an individual fixture to be small enough that its intended use should be clear. If they start to get too big then it should be easy enough to add a whitelist on each instance as appropriate.

Copy link
Member

@xavfernandez xavfernandez left a comment

Choose a reason for hiding this comment

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

Did you measure the speedup for tests/functional/test_completion.py & tests/functional/test_list.py ?

@chrahunt
Copy link
Member Author

chrahunt commented Nov 2, 2019

On my machine™, with times averaged over 5 runs, it looks like:

file cores master (s) new (s)
completion 1 13.4762 8.7662
2 7.8660 6.6554
list 1 51.3596 39.8602
2 26.8778 22.6406

Progress on pytest-dev/pytest-xdist#18 would probably improve this some (especially if the files are executed at the same time).

@chrahunt chrahunt merged commit 39d16b6 into pypa:master Nov 2, 2019
@chrahunt chrahunt deleted the refactor/shared-test-scripts branch November 2, 2019 15:45
@chrahunt
Copy link
Member Author

chrahunt commented Nov 2, 2019

Thanks for reviewing @xavfernandez!

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Dec 2, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation C: tests Testing and related things skip news Does not need a NEWS file entry (eg: trivial changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants