Add threshold check on number of tests executed

[holgerd77]

Taking up on this comment along the pre-Berlin ethereum/tests run:

We should add a simple threshold check on the number of executed state and blockchain tests. This would avoid the situation where number of executed tests implicitly dropped (e.g. by a change on the ethereum/tests test folder setup and no one noticed since CI still passed with the reduced number of tests.

Threshold can just be a reasonable number from real life, eventually this can be set to the exact number of the respective tests along HF and test type lines, normally test number just increase over time. Alternatively this can be set somewhat below.

On test runs executed less tests than set as threshold, test run should visibly fail in CI.

[holgerd77]

I did some counting of test case numbers and compared the run from #743 (current state of test repo + VM) with a run from 2019 May 13 along this (randomly chosen, just for the sake of being some substantial time in the past) and coming to - ahem - interesting numbers (first ones without explicit mentioning are the state test numbers):

Hardfork	2019 May 13	Current State
Byzantium	4762	2272
Constantinople	10536	2345
Petersburg	10531	2340
Istanbul	---	2377
MuirGlacier	---	2377
BlockchainTests (Petersburg / Istanbul)	936	> 40.000

Hmm. 😄 This might actually need some closer look.

[holgerd77]

One way of generally addressing this (without solving eventual concrete number discrepancies here on this run but just prevent off-droppings in the future) would be to allow (this needs to be optional for convenience reasons) to pass some parameter on the command line like:

npm run test:state -- --fork=Petersburg --expected-test-runs=2340

This could then be used within the package.json test commands.

Since this is predictable this wouldn't cause any hazzle on everyday runs. At the same time it is naturally enforced that one is looking at the changed test case numbers on update PRs of the ethereumjs-testing dependency and one would then automatically stumble upon inconsistencies along.

Would actually be in favor of the solution. What do you think?

[evertonfraga]

Interesting! :D
The numbers are a bit shocking, there must be a good explanation for that huge drop.

[evertonfraga]

Oh, and your proposed solution is a great and simple idea.

[jochem-brouwer]

Can indeed add this CLI flag. This would be pretty specific for the blockchain tests on the CI though, as we run those on both only the "slow directory" and the other directories for blockchain tests.

[holgerd77]

Can't follow the argumentation here TBH, why can't we use this for the state tests e.g. as stated in the example from above? So the idea is just to take the number of test executions from one (latest e.g.) CI run and then add this as a fixed number to the CL parameter to from then on be "notified" (in the sense of: test run fails) when there is a deviation from that number for whatever reason.

[jochem-brouwer]

Yep you are right.

[jochem-brouwer]

Closed by #849