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.

Sign up for GitHub

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

Jump to bottom

Use deterministic order and naming of unit tests #4787

Closed
pavoljuhas opened this issue Dec 30, 2021 · 1 comment
Closed

Use deterministic order and naming of unit tests #4787

pavoljuhas opened this issue Dec 30, 2021 · 1 comment
Labels
kind/feature-request Describes new functionality

Comments

@pavoljuhas
Copy link
Collaborator

Is your feature request related to a use case or problem? Please describe.

Checking at 4ebfb1c the unit test collection is not reproducible, because UT parameters depend on random numbers and value order in sets / dicts. As a result, it is not possible to execute tests in parallel using the -n N, --numprocesses=N option, because pytest first checks if all worker jobs have the same collection of tests. This fails, because test names are different in each job:

$ git checkout 4ebfb1c8fa123ec6164ff8102b69858ac868840f
$ check/pytest -n2
======================================= test session starts ========================================
platform linux -- Python 3.9.9, pytest-6.2.5, py-1.10.0, pluggy-1.0.0
rootdir: /home/user/Code/gh/quantumlib/Cirq
plugins: xdist-2.2.1, forked-1.3.0, asyncio-0.16.0, cov-3.0.0
gw0 [15840] / gw1 [15840]
collecting 0 items / 1 error
============================================== ERRORS ==============================================
_______________________________________ ERROR collecting gw0 _______________________________________
Different tests were collected between gw1 and gw0. The difference is:
...

This irreproducibility makes it also trickier to re-run failing unit tests if their parameters are random.

Describe the solution you'd like

Replace parameters in sets or dicts with fixed-order containers or sort them before test parametrization.
Use deterministic seeding for random number generation when running unit tests.

[optional] Describe alternatives/workarounds you've considered

[optional] Additional context (e.g. screenshots)

What is the urgency from your perspective for this issue? Is it blocking important work?

P1 - I need this no later than the next release (end of quarter)
@pavoljuhas pavoljuhas added the kind/feature-request Describes new functionality label Dec 30, 2021
@pavoljuhas pavoljuhas mentioned this issue Dec 30, 2021
Merged
CirqBot pushed a commit that referenced this issue Jan 7, 2022
@pavoljuhas
Make test ordering and parametrization reproducible (#4788)
45e7ddd 
Implement deterministic ordering of pytest parameters.
Add CIRQ_TESTING_RANDOM_SEED environment variable for seeding
random numbers generation in random and numpy.random modules.
Use commit epoch time as the default seed to have a non-constant,
but deterministic seeding.

NB: Use empty CIRQ_TESTING_RANDOM_SEED to prevent seeding, for example,
    CIRQ_TESTING_RANDOM_SEED="" check/pytest

This implements #4787
@pavoljuhas
Copy link
Collaborator Author

Fixed by #4788.

MichaelBroughton pushed a commit to MichaelBroughton/Cirq that referenced this issue Jan 22, 2022
@pavoljuhas @MichaelBroughton
Make test ordering and parametrization reproducible (quantumlib#4788)
0824992 
Implement deterministic ordering of pytest parameters.
Add CIRQ_TESTING_RANDOM_SEED environment variable for seeding
random numbers generation in random and numpy.random modules.
Use commit epoch time as the default seed to have a non-constant,
but deterministic seeding.

NB: Use empty CIRQ_TESTING_RANDOM_SEED to prevent seeding, for example,
    CIRQ_TESTING_RANDOM_SEED="" check/pytest

This implements quantumlib#4787
@pavoljuhas pavoljuhas mentioned this issue Sep 8, 2022
Merged
CirqBot pushed a commit that referenced this issue Sep 20, 2022
@pavoljuhas
Use pytest-randomly for reproducible random test parameters (#5868)
8ff2ce0 
- Require pytest-randomly for the testing to ensure RNG-generated test
  parameters are consistent across parallel test jobs.
- Remove check/pytest option --actually-quiet which disables pytest-randomly
  hook for seeding parallel test jobs.
- Remove `CIRQ_TESTING_RANDOM_SEED` as it is not needed anymore.
- Remove RNG seeding in the tests where it seems redundant.
- Clean up one instance of unnecessary test parametrization.

This upholds #4787 and replaces #4788.
Reverts #1825 and obsoletes #1826.
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
@pavoljuhas @rht
Make test ordering and parametrization reproducible (quantumlib#4788)
b15ef56 
Implement deterministic ordering of pytest parameters.
Add CIRQ_TESTING_RANDOM_SEED environment variable for seeding
random numbers generation in random and numpy.random modules.
Use commit epoch time as the default seed to have a non-constant,
but deterministic seeding.

NB: Use empty CIRQ_TESTING_RANDOM_SEED to prevent seeding, for example,
    CIRQ_TESTING_RANDOM_SEED="" check/pytest

This implements quantumlib#4787
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
@pavoljuhas @rht
Use pytest-randomly for reproducible random test parameters (quantuml…
1299977 
…ib#5868)

- Require pytest-randomly for the testing to ensure RNG-generated test
  parameters are consistent across parallel test jobs.
- Remove check/pytest option --actually-quiet which disables pytest-randomly
  hook for seeding parallel test jobs.
- Remove `CIRQ_TESTING_RANDOM_SEED` as it is not needed anymore.
- Remove RNG seeding in the tests where it seems redundant.
- Clean up one instance of unnecessary test parametrization.

This upholds quantumlib#4787 and replaces quantumlib#4788.
Reverts quantumlib#1825 and obsoletes quantumlib#1826.
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
@pavoljuhas
Make test ordering and parametrization reproducible (quantumlib#4788)
f40cb10 
Implement deterministic ordering of pytest parameters.
Add CIRQ_TESTING_RANDOM_SEED environment variable for seeding
random numbers generation in random and numpy.random modules.
Use commit epoch time as the default seed to have a non-constant,
but deterministic seeding.

NB: Use empty CIRQ_TESTING_RANDOM_SEED to prevent seeding, for example,
    CIRQ_TESTING_RANDOM_SEED="" check/pytest

This implements quantumlib#4787
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
@pavoljuhas
Use pytest-randomly for reproducible random test parameters (quantuml…
e8ea0c4 
…ib#5868)

- Require pytest-randomly for the testing to ensure RNG-generated test
  parameters are consistent across parallel test jobs.
- Remove check/pytest option --actually-quiet which disables pytest-randomly
  hook for seeding parallel test jobs.
- Remove `CIRQ_TESTING_RANDOM_SEED` as it is not needed anymore.
- Remove RNG seeding in the tests where it seems redundant.
- Clean up one instance of unnecessary test parametrization.

This upholds quantumlib#4787 and replaces quantumlib#4788.
Reverts quantumlib#1825 and obsoletes quantumlib#1826.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/feature-request Describes new functionality
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@pavoljuhas