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

Add new decorators to skip tests and handle it in testgen #2016

Merged
merged 4 commits into from Sep 1, 2020

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Aug 13, 2020

Sorry, 3 different parts in this PR.

Issue 1 (minor)

get_custody_secret is duplicate.

Solution for Issue 1

Use v-guide's get_custody_secret in test helper.


Issue 2

For some test cases, it can't be passed with mainnet configs.

Solution for Issue 2

  1. Add with_configs decorator to assign available configs
  2. Add only_full_crosslink decorator to detect if the configuration can
    do full crosslinking: some test cases assume that each slot has enough committee to form crosslink.
  3. Add CONFIG_NAME to the config files. "mainnet" or "minimal".

Issue 3

  1. Test generator calls test case without pytest.
  2. Devs and CI use pytest to run the tests.

It would be better to handle the "unavailable" forks and configs properly. e.g., some test cases are only for phase 0, or some test cases are only for mainnet configs.

Solution for Issue 3

  1. Add context.is_pytest flag: True if calling via pytest. False if calling from test generator.

  2. Add dump_skipping_message to dump message to pytest caller or test generator runner:

    def dump_skipping_message(reason: str) -> None:
        message = f"[Skipped test] {reason}"
        if is_pytest:
            pytest.skip(message)
        else:
            raise SkippedTest(message)
    • If it's for pytest: mark it is skip. For example, if we call pytest --disable-bls --config=mainnet eth2spec/test/phase1/sanity/test_blocks.py, it will show some test cases are skipped:

    image

    • If it's called by test generator, the SkippedTest exception will be caught and show message like:
    Generating test: output/minimal/phase1/sanity/blocks/pyspec_tests/multiple_different_validator_exits_same_block
    [Skipped test] doesn't support this fork: phase1
    
  3. Add pytest to gen_helpers requirements. But note that it's just because pytest is imported in eth2spec/test/context.py, the test generator does NOT use it.

1. Add `with_configs` decorator to assign available configs
2. Add `only_full_crosslink` decorator to detect if the configuation can
do full crosslinking
3. Add `context.is_pytest` flag: True if calling via pytest. False if
calling from test generator.
Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

well done!

tests/core/gen_helpers/requirements.txt Outdated Show resolved Hide resolved
Copy link
Collaborator

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

Nice work! Changes look good, but there are a few older tests where the test had to be skipped. Updating these to raise the new SkippedTest would be best I think.

tests/core/gen_helpers/gen_base/gen_runner.py Show resolved Hide resolved
Copy link
Collaborator

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

LGTM

@protolambda protolambda merged commit 8de7f95 into testgenphase1 Sep 1, 2020
@protolambda protolambda deleted the hwwhww/phase1-tests branch September 1, 2020 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants