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

Spectest decorator #1052

Merged
merged 25 commits into from
May 12, 2019
Merged

Spectest decorator #1052

merged 25 commits into from
May 12, 2019

Conversation

protolambda
Copy link
Collaborator

@protolambda protolambda commented May 5, 2019

Since the pytests share a lot of "edge case" setups with test-generation, I'm looking to unify these tests with the generator code, i.e. not duplicate testing work.

To achieve this, a few things have to be changed:

  • generators don't know any "fixtures" like pytests do. However, we can implement an elegant decorator utility to provide test functions with things like a genesis-state setup. I included an implementation for this in context.py/utils.py: with_args (generalized) and with_state (built on top of generalized version, provides fresh genesis state).
  • existing tests have to be ported over. Luckily, the format introduced here is similar, yet shorter (yield from is a blessing) and faster (less, if not none, deep-copies)

The new format:

We have a new decorator for tests: @spectest(description='very informative') (description is optional)
This decorator enables you to:

  • call the test-function with an optional generator_mode param, when True it makes the test function collect yielded data into a dict, incl. description, and return it.
  • ignore yields completely, and execute the full function like a normal test, when not in generator_mode (default).

Example test:

@spectest()
def test_foobar_thing():
  state = ... (preparation)
  state.special_thing = 123 # edge case set up
  yield 'pre', state
  operation = ...
  yield 'op', operation
  process_thing(state, operation)
  assert state.special_thing > 300 # assert changes like normal test
  yield 'post', state

Note that in some cases, some of it can be generalized with a local helper function, and then you just call yield from run_processing_foobar(state, operation) or similar. See POC attestations test update.

And, you can call the test to create a test-case vector:

test_case = test_foobar_thing(generator_mode=True)

# returns:
{
  'description': 'foobar_thing'
  'pre': ...,
  'op': ...,
  'post': ...
}

Note that values yielded in generator mode are encoded before continuing in the function, so there's no deep-copy necessary: after encoding the original containers may mutate.
This also speeds up the normal test-runs, as values are not collected/encoded, nor deep-copied, so you get the minimal amount of object instances / memory allocation / etc.

Some syntax sugar

99% of the pytests look like this:

@with_state
@spectest()
def test_my_thing(state):
   ...

To make this shorter, we define a @spec_state_test that composes these two generators. It's literally a state-spec-test!

from tests.context import spec_state_test

@spec_state_test
def test_my_thing(state):
   ...

For anyone interested in previous iterations of the edge-case unification idea, here they are: https://notes.ethereum.org/5N7MrvOESrKK3AIHYYvYww#
The important pivot made here is that we do not separate the edge-cases from testing, nor do we reference the processing function, and make the decorator run the behavior. This enables more freedom in testing, yet it is powerful enough to create test-vectors cleanly from the existing pytest work.

Next steps:

  • update remaining tests
  • make block-operation test-generator and epoch transition test-generator call these tests, and output vectors into spec-tests repo.

@protolambda protolambda changed the base branch from dev to v06x May 6, 2019 16:39
@djrtwo
Copy link
Contributor

djrtwo commented May 6, 2019

ported all remaining tests.
Took a shortcut on finality tests. Did a bit of cleaning but kept the one deepcopy where it was to make comparison between pre and post state each epoch easier.

@djrtwo
Copy link
Contributor

djrtwo commented May 7, 2019

whoops on the extra array nesting. nice catch

@protolambda
Copy link
Collaborator Author

Tests are running, and I got block-operation generators to work based on the same tests 🎉
I think we can merge this in after review, and then we update the remaining part of test-generation later to call the epoch-transition tests, sanity tests etc. I don't want this to get too much out of sync with the dev tests to avoid large merge conflicts / people writing outdated style tests.

@protolambda protolambda marked this pull request as ready for review May 8, 2019 16:31
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.

In general really happy with it. The main thing missing is documenting usage, but that can come once we do another pass and add other test types.

Just a few very minor comments.

We should cut a v0.6.1-t0 release to master and backport this into dev

test_generators/operations/main.py Outdated Show resolved Hide resolved
test_generators/operations/main.py Outdated Show resolved Hide resolved
test_generators/operations/suite_creator.py Outdated Show resolved Hide resolved
Co-Authored-By: Danny Ryan <dannyjryan@gmail.com>
@protolambda
Copy link
Collaborator Author

Cleaned up PR suggestion commits

Co-Authored-By: jannikluhn <jannik@brainbot.com>
@protolambda
Copy link
Collaborator Author

3189cf0 includes the new edge cases presented in #1010, in a more concise form, as part of the unified spec-tests. Thanks @jannikluhn. Hope to write many more spectests in this format with the other testers, sorry about the delay getting the tests in because of this PR.

protolambda and others added 3 commits May 11, 2019 17:51
Co-Authored-By: Dmitry S <zilm13@users.noreply.github.com>
@protolambda
Copy link
Collaborator Author

b4be220 includes new attestation edge cases presented in #999. Thanks @zilm13. See above comment, the format is improving, and spec testing shares effort with test-vector generation now. Sorry about the delay of getting test cases in, hope it's worth it :)

@djrtwo
Copy link
Contributor

djrtwo commented May 12, 2019

merging.
testing work continued on separate branches such as spectest-deco PR 1052

@djrtwo djrtwo merged commit ca22c19 into v06x May 12, 2019
@djrtwo djrtwo deleted the spectest-deco branch May 12, 2019 14:58
@djrtwo djrtwo mentioned this pull request May 20, 2019
4 tasks
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

2 participants