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

pytests use configuration system #979

Merged
merged 10 commits into from
Apr 24, 2019
Merged

pytests use configuration system #979

merged 10 commits into from
Apr 24, 2019

Conversation

protolambda
Copy link
Collaborator

@protolambda protolambda commented Apr 22, 2019

  • split requirements.txt in two: one for normal use (whatever that may be), one for running tests (has the normal requirements.txt as -r line to ensure you don't miss the dependencies)
  • add command option to conftest to switch
  • update makefile

Also: I fixed a minor testing bug that was uncovered because of a small constant change: minimal config has a shorter eth1-data voting period. The new winning vote set the eth1-deposit count to 0, and it resulted in a negative expected-deposit-count. Default test-blocks are prepared with a vote for whatever deposit count the state says it is at.
Maybe we should add a voting requirement to the spec itself, so that you can't make the eth1-data deposit counter lower than the deposit index in the state.

Note: I don't believe anyone has actively been maintaining the test-correctness for non-minimal_config. Currently, when running tests against the mainnet config (slower), some of them fail. I identified a few root causes: assert verify_attestation(...) fails for some reason, a committee for some shard was not found, and committee size <-> bitfield size is off.

@hwwhww
Copy link
Contributor

hwwhww commented Apr 22, 2019

I made another PR on it: #981 so that we can organize all dependencies in setup.py. 😄

@protolambda
Copy link
Collaborator Author

rebased on dev, PR should run cleanly now. Hope this resolves the strange local-execution issues @hwwhww was experiencing.

@hwwhww
Copy link
Contributor

hwwhww commented Apr 24, 2019

It works now! Straaange.

@djrtwo
Copy link
Contributor

djrtwo commented Apr 24, 2019

I don't believe anyone has actively been maintaining the test-correctness for non-minimal_config

These failed tests had to do with the low number of validators (100) compared to the slots_per_epoch. When generating attestations, I am often assuming there is at least 2 validators per committee. I changed num_validators to scale with slots_per_epoch and tests are fixed. but now the mainnet tests take a very long time to run.

@djrtwo djrtwo merged commit 90575cc into dev Apr 24, 2019
@djrtwo djrtwo deleted the pytest-use-config branch April 24, 2019 18:35
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