test: only skip ssz_static tests associated to missing type #6798
Conversation
| @@ -57,25 +57,28 @@ const sszStatic = | |||
| (ssz.altair as Types)[typeName] || | |||
| (ssz.phase0 as Types)[typeName]; | |||
|
|
|||
| it(`SSZ type ${typeName} for fork ${fork} is defined`, function () { | |||
| expect(sszType).toBeDefined(); | |||
There was a problem hiding this comment.
Can you specify an error message in expect()? Currently it throws AssertionError: expected undefined not to be undefined which doesn't make too much sense unless we look at the name of the test.
Something like
expect(sszType, `SSZ type ${typeName} for fork ${fork} is not defined`).toBeDefined()
and
it(`${fork} - ${typeName} existence check`)
There was a problem hiding this comment.
Can you specify an error message in expect()?
This works but gives an eslint error
Expect takes most 1 argument eslint (vitest/valid-expect)
We have to use the custom function toEqualWithMessage in that case
There was a problem hiding this comment.
AssertionError: expected undefined not to be undefined which doesn't make too much sense unless we look at the name of the test.
Yes I totally agree with this but @nazarhussain has been pushing against having detailed assertion error messages so I kept it as is.
Right now if you wanna have a better assertion message you either have to
- use
toBeWithMessage/toEqualWithMessagewhich is crossed out as it is marked deprecated - or pass as 2nd param to expect and get an eslint error
We might wanna rethink this, imo it would be best if we override the vitest default toBe and toEqual to support the message as a 2nd param, and remove the custom toBeWithMessage and toEqualWithMessage
There was a problem hiding this comment.
I tend to agree with @nazarhussain on this. The spec test name should be descriptive enough to explain what is being asserted, and there should ideally be only one expect in each test case so that its clear which assertion is failing. I tend to feel that adding more messaging to the test case means that the naming or assertion is incorrect or confusing and should be refactored
There was a problem hiding this comment.
Maybe I am nitpicking or not used how vitest outputs logs, but when I debug I first look at the message in red, which in this case is not descriptive at all. I will then need to look at the unit test name, which is white and is prepended by file name and spec test name.
On the other hand, this is good because when I see the red message, I know `deposit_requests_root` is missing right from the first glance
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
…e#6798) * test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
* test: only skip ssz_static tests associated to missing type * More detailed error message if type is not defined
|
🎉 This PR is included in v1.22.0 🎉 |
Motivation
Motivation from #6776 to only skip ssz_static tests associated to missing type instead of all ssz_static tests.
Description
it-statement to assert that type is defined