Skip to content

import Spec objects for each test case#2755

Closed
hwwhww wants to merge 1 commit intoex-ante-testsfrom
specs-import
Closed

import Spec objects for each test case#2755
hwwhww wants to merge 1 commit intoex-ante-testsfrom
specs-import

Conversation

@hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Dec 1, 2021

Issue

  1. spec_configured_state_test is not thread-safe:
    • The given spec: Spec object was initialized with global spec_targets in context.py.
    • spec_configured_state_test overrides spec.config, execute the test case, and then change it back to the unchanged config.
    • In multi-processing or multi-thread context, if a config field is overridden when running one test case, the spec.config object in other concurrent tasks might be changed too.
  2. pytest-xdist implemented parallelization with subprocess.
  3. We want to make the test generator parallelable!

Due to 1+2, #2752 failed because some of the ex-ante tests override spec.config.PROPOSER_SCORE_BOOST temporarily, and then concurrently, test_proposer_boost_correct_head failed because it wrongly used the overridden value.

How did I fix it

To better isolate the test cases, it would be better to reduce the risk of these side effects. This PR tries to import Spec objects for each test case. The solution was found from here and here.

@hwwhww hwwhww added the testing CI, actions, tests, testing infra label Dec 1, 2021
@hwwhww hwwhww requested review from djrtwo and ralexstokes December 1, 2021 15:35
@hwwhww hwwhww mentioned this pull request Dec 1, 2021
3 tasks
module = importlib.util.module_from_spec(module_spec)
loader = importlib.util.LazyLoader(module_spec.loader)
module_spec.loader = loader
import sys
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: hoist import sys to the top of the file

@hwwhww
Copy link
Contributor Author

hwwhww commented Dec 7, 2021

replaced by #2765

@hwwhww hwwhww closed this Dec 7, 2021
@ralexstokes ralexstokes deleted the specs-import branch December 7, 2021 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing CI, actions, tests, testing infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants