elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.65k stars 8.23k forks source link

Test `is superseded by migration errors and does not run if a migration fails` is dependent on branch version #127195

Closed mistic closed 2 years ago

mistic commented 2 years ago

I think we should try to rewrite the test is superseded by migration errors and does not run if a migration fails at src/core/server/saved_objects/validation/integration_tests/validator.test.ts to not be dependent on the version of the branch otherwise it implies this file needs to be changed at each version bump.

elasticmachine commented 2 years ago

Pinging @elastic/kibana-core (Team:Core)

pgayvallet commented 2 years ago

We're using the kibana version in the declarations

https://github.com/elastic/kibana/blob/b7e366c483016f63b1b1d0598adff2c7037e3dc3/src/core/server/saved_objects/validation/integration_tests/validator.test.ts#L109-L113

but not in the snapshot:

https://github.com/elastic/kibana/blob/b7e366c483016f63b1b1d0598adff2c7037e3dc3/src/core/server/saved_objects/validation/integration_tests/validator.test.ts#L193-L195

We need to replace the snapshot-based assertion with something using the actual version number (or use string interpolation in the snapshot, but that's kinda bad when running with -u)

afharo commented 2 years ago

Probably just matching the error message to a text instead of a snapshot?

}).rejects.toThrowError(`Migration function for version ${version} threw an error`); 
pgayvallet commented 2 years ago

Yup, that was

or use string interpolation in the snapshot, but that's kinda bad when running with -u

It's fine, the issue is that any snapshot update will override the custom string. But we can just be careful about that.