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

Remove concurrency handling from Makefile stestr usage #10097

Merged
merged 1 commit into from
May 10, 2023

Conversation

mtreinish
Copy link
Member

Summary

In #1973 the Makefile was updated to add manual concurrency handling to the invocation of stestr. At the time (> 4 yrs ag) our CI setup looked very different, everything was run in TravisCI which had much smaller memory quotas in the worker VMs and we also were actively using the Makefile to run tests. Since that time though CI and our development workflow have changed. At the time running multiple tests in parallel was causing us to exhaust the available RAM in the CI workers for MacOS and Windows so we set concurrency limits to prevent this. However, we no longer use travis, and the use of the makefile has long since been replaced by using tox as it provides a unified interface that manges venvs along with running tests in a consistent way across systems (including locally and CI). This commit reverts that change and lets stestr run with it's default concurrency when people use the Makefile. While the Makefile isn't widely used and has been largely superseded by tox, there are still some people using it and of those using MacOS or Windows this should improve local test throughput.

Details and comments

In Qiskit#1973 the Makefile was updated to add manual concurrency handling to
the invocation of stestr. At the time (> 4 yrs ag) our CI setup looked
very different, everything was run in TravisCI which had much smaller
memory quotas in the worker VMs and we also were actively using the
Makefile to run tests. Since that time though CI and our development
workflow have changed. At the time running multiple tests in parallel
was causing us to exhaust the available RAM in the CI workers for MacOS
and Windows so we set concurrency limits to prevent this. However, we no
longer use travis, and the use of the makefile has long since been replaced
by using tox as it provides a unified interface that manges venvs along
with running tests in a consistent way across systems (including locally
and CI). This commit reverts that change and lets stestr run with it's
default concurrency when people use the Makefile. While the Makefile isn't
widely used and has been largely superseded by tox, there are still some
people using it and of those using MacOS or Windows this should improve
local test throughput.
@mtreinish mtreinish requested a review from a team as a code owner May 10, 2023 15:06
@mtreinish mtreinish added type: qa Issues and PRs that relate to testing and code quality Changelog: None Do not include in changelog labels May 10, 2023
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@coveralls
Copy link

Pull Request Test Coverage Report for Build 4938541775

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 16 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.04%) to 85.891%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 4 91.14%
crates/qasm2/src/parse.rs 12 97.11%
Totals Coverage Status
Change from base Build 4937371425: -0.04%
Covered Lines: 71011
Relevant Lines: 82676

💛 - Coveralls

Copy link
Contributor

@kevinhartman kevinhartman left a comment

Choose a reason for hiding this comment

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

LGTM.

@kevinhartman kevinhartman added this pull request to the merge queue May 10, 2023
Merged via the queue into Qiskit:main with commit 2b18980 May 10, 2023
@mtreinish mtreinish deleted the juliens-tests-are-slow branch May 10, 2023 18:57
king-p3nguin pushed a commit to king-p3nguin/qiskit-terra that referenced this pull request May 22, 2023
In Qiskit#1973 the Makefile was updated to add manual concurrency handling to
the invocation of stestr. At the time (> 4 yrs ag) our CI setup looked
very different, everything was run in TravisCI which had much smaller
memory quotas in the worker VMs and we also were actively using the
Makefile to run tests. Since that time though CI and our development
workflow have changed. At the time running multiple tests in parallel
was causing us to exhaust the available RAM in the CI workers for MacOS
and Windows so we set concurrency limits to prevent this. However, we no
longer use travis, and the use of the makefile has long since been replaced
by using tox as it provides a unified interface that manges venvs along
with running tests in a consistent way across systems (including locally
and CI). This commit reverts that change and lets stestr run with it's
default concurrency when people use the Makefile. While the Makefile isn't
widely used and has been largely superseded by tox, there are still some
people using it and of those using MacOS or Windows this should improve
local test throughput.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog type: qa Issues and PRs that relate to testing and code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants