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

Speed up tests execution #1963

Merged
merged 5 commits into from
Aug 20, 2023
Merged

Conversation

atugushev
Copy link
Member

@atugushev atugushev commented Aug 10, 2023

Session-scoped fixtures

The fake_dists fixture is one of the most time-consuming fixtures, so it makes sense to make it and all related fixtures session-scoped.

Finetune tests distribution over the run

Initially, tests are distributed evenly across all worker processes. But during the execution of tests, some processes may finish running tests faster than others. In these cases, the --dist=worksteal option allows these "free" workers to "steal" tests from the queue of other, still working, processes.

This is particularly useful if the run times of different tests vary greatly, as this distribution method ensures more efficient resource usage, improving the performance of testing.

Refactored slow tests

test_generate_hashes_with_split_style_annotations and test_generate_hashes_with_line_style_annotations were utilizing too many external packages. We should always prefer local packages as they are faster and pip-compile doesn't need network to compile them.

Stats

Locally, it increases speed by about 30%. Here are CI runs:

Screenshot 2023-08-10 at 21 47 56

t's better to use local packages for tests instead of
external packages that hit the network.
@atugushev atugushev closed this Aug 10, 2023
@atugushev atugushev reopened this Aug 10, 2023
@atugushev
Copy link
Member Author

atugushev commented Aug 10, 2023

FTR, I'll reopen a few times to gather the CI stats.

@atugushev atugushev closed this Aug 10, 2023
@atugushev atugushev reopened this Aug 10, 2023
@atugushev atugushev closed this Aug 10, 2023
@atugushev atugushev reopened this Aug 10, 2023
Copy link
Member

@theryanwalker theryanwalker left a comment

Choose a reason for hiding this comment

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

Looks great!

@atugushev
Copy link
Member Author

@theryanwalker #1967 should fix failing CI.

@atugushev atugushev enabled auto-merge (squash) August 20, 2023 10:07
@atugushev atugushev merged commit af7f6ef into jazzband:main Aug 20, 2023
34 checks passed
apljungquist added a commit to apljungquist/pip-tools that referenced this pull request Aug 20, 2023
The test is failing locally since "Speed up tests execution (jazzband#1963)"
but the xfail should not be part of the PR.
@@ -99,6 +99,7 @@ disallow_untyped_defs = false
addopts = [
# `pytest-xdist`:
"--numprocesses=auto",
"--dist=worksteal",
Copy link
Member

Choose a reason for hiding this comment

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

Such settings would benefit from having an inline code comment with the motivation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! Addressed in #1973

@webknjaz
Copy link
Member

@atugushev one other thing I used to do in the past, was splitting the tests across multiple CI jobs so that it adds parallelism in an extra dimension. We could consider doing the same here.

@atugushev atugushev deleted the speedup-tests-v2 branch August 20, 2023 23:01
@atugushev
Copy link
Member Author

one other thing I used to do in the past, was splitting the tests across multiple CI jobs so that it adds parallelism in an extra dimension. We could consider doing the same here.

@webknjaz, that's an interesting idea. Do you have any suggestions in mind regarding pip-tools?

@atugushev atugushev added the tests Testing and related things label Aug 20, 2023
@webknjaz
Copy link
Member

Just a thought. It can be implemented as a pytest option in a few lines of code. But it might be not worth it, depending on the overhead of spawning more VMs. This really needs testing to see if it makes things better or worse.

apljungquist added a commit to apljungquist/pip-tools that referenced this pull request Oct 15, 2023
Also fix some docstrings that were not valid rst.

Include build-system hook in via comments

Use meaningful names for fake build deps

Install static deps before requesting dynamic deps

Address review feedback

Remove python 3.7 from tox.ini

This should probably have been done in 51d9e1c (Drop support for
Python 3.7).

Use default labels for links

Add test coverage for Dependency

Introduce piptools.build module for building project metadata

Fix naming

Fixes for checkqa

Address feedback

Also
* remove unused `src_file` argument
* use filename as comes_from to avoid getting absolute paths in the
  output
* remove unused small-fake-c from fake_dists_with_build_deps
* mark tests as backtracking_resolver_only since the result is
  independent of resolver

Use Any type because mypy gives inconsistent results

Address feedback (II)

Assert exit code separately from stdout

Use intermediate variable

Move 'may be used more than once' to end of help text

Rename build-deps-only > only-build-deps

Use ALL_BUILD_DISTRIBUTIONS from already imported options

Use relative file path in comes from

Remove xfail

The test is failing locally since "Speed up tests execution (jazzband#1963)"
but the xfail should not be part of the PR.

Make pip-compile silent by default

Improve type hints in build

Remove excessive comments

Give build distribution literal a meaningful name

Test that transient build deps are excluded

Address feedback and adapt for build>1

Fix checkqa

Use build target terminology consistently

Use pathlib.Path consistently in build.py and new tests

More specific type hinting in build.py

Don't convert to absolute path unnecessarily

Update help texts

* Use "extract" instead of "install" in help texts
* Align texts that said different things but probably should be the same

Import Monkeypatch from public API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Avoid listing in changelog tests Testing and related things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants