Skip to content

Conversation

jonathanberthias
Copy link
Contributor

The test dependencies are repeated multiple times in the GitHub workflows, and they must be manually installed to run the tests locally.

This PR adds a dependency group to make installing all required dependencies as easy as:

pip install --group test

I also added pytest-xdist to speed tests up as suggested by @Zeroto521. On my laptop, the sequential tests complete in 1m30s, whereas with -n auto with 8 cores, they only take 30s, such that running the entire test suite is 3x faster!

Closes #1079

@Joao-Dionisio Joao-Dionisio requested a review from Copilot October 10, 2025 09:19
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a PEP 735 dependency group for test dependencies to simplify test environment setup and adds pytest-xdist for parallel test execution to improve performance.

  • Added a test dependency group in pyproject.toml containing all test-related dependencies
  • Updated GitHub workflows to use the new dependency group instead of manually listing packages
  • Added pytest-xdist for parallel test execution with -nauto flag

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pyproject.toml Added test dependency group with test-related packages
docs/build.rst Updated testing instructions to use dependency group
INSTALL.md Updated installation instructions for test dependencies
CHANGELOG.md Added changelog entry for the new dependency group
.github/workflows/*.yml Updated workflows to use dependency group and parallel testing
.coveragerc Added patch configuration for subprocess

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Joao-Dionisio
Copy link
Member

Thank you @jonathanberthias!
Can you just confirm that the dependencies in the dependency group you added were all already dependencies when you ran the tests locally, except for pytest-xdist? Doesn't seem to make sense to have coverage as a default dependency when running tests locally.

@jonathanberthias
Copy link
Contributor Author

Thank you @jonathanberthias! Can you just confirm that the dependencies in the dependency group you added were all already dependencies when you ran the tests locally, except for pytest-xdist? Doesn't seem to make sense to have coverage as a default dependency when running tests locally.

The dependencies I used are those that were installed in the coverage workflow:

python -m pip install networkx cython pytest-cov numpy

The main difference is I used coverage directly instead of pytest-cov since the calls below use coverage run ....

I agree that for most users, installing coverage is probably not needed, though I don't think it does much harm to install one extra dependency (same can be said about networkx, it's a big dependency for just a couple tests). But of course, I can remove coverage from the dependency group and explicitly install it in the workflow. Just tell me what you prefer 🙏

@Joao-Dionisio
Copy link
Member

Joao-Dionisio commented Oct 11, 2025

@jonathanberthias we're generally in favor of keeping things as light as possible (there was a big discussion on whether we should add numpy as a dependency for matrix variables). I'd be more inclined to keep it that way in the tests as well, for consistency.

So, would you mind doing what you suggest with coverage and networkx, please?

@jonathanberthias
Copy link
Contributor Author

Yes good idea, I added a separate group for full coverage with networkx and coverage, and the "standard" test group only has pytest and pytest-xdist for dev use.

One remaining is question relates to the integration tests, at the moment they only install the test group so they skip the tests that depend on networkx. Is that ok? Or should I change it so that the integration tests run all the tests?

@Joao-Dionisio
Copy link
Member

@jonathanberthias Hmmm I will say that it makes sense for all tests to be run, which also includes installing itertools in the full coverage. It's used in test_tsp and test_logical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: Speed up test via pytest-xdist

2 participants