test: improve ssz tests consistency - #6776
Conversation
| (ssz.altair as Types)[typeName] || | ||
| (ssz.phase0 as Types)[typeName]; | ||
| if (!sszType) { | ||
| throw Error(`No type for ${typeName}`); |
There was a problem hiding this comment.
doesn't this just result in strange errors downstream if we don't throw here and exit the test run?
There was a problem hiding this comment.
Makes sense. I updated so that when sszType is null, associated tests are not run at all; also improved the error message to be more meaningfull.
Compared to the original Error based approach, a missing SSZ type will not prevent further tests to be run (the Error would short-circuit the logic)
nflaig
left a comment
nflaig
left a comment
There was a problem hiding this comment.
Still not sure this improves the situation, could you provide the output of running the ssz_static tests with a missing ssz type before vs. after your changes
|
The main thing that it improves, besides better error message giving some context, is that further tests are not short-circuited. |
I understand that but can you provide a before vs. after example |
wemeetagain
left a comment
wemeetagain
left a comment
There was a problem hiding this comment.
Makes sense to me.
The test count is still variable, but at least one missing ssz type won't cause an unknown number of unrelated ssz type tests from being suppressed.
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
* test: improve ssz tests consistency * chore: address comments
|
馃帀 This PR is included in v1.22.0 馃帀 |
3 participants
Motivation
Make sure all tests always run
Description
Do not throw an error when an SSZ type is missing, but rather fail a test.
This let subsequent generated tests run not matter what, ensuring the tests count is consistent.