-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Enhance Tests around SnapshotInfo UserMetadata #74362
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
Enhance Tests around SnapshotInfo UserMetadata #74362
Conversation
We barely test the correct handling of user metadata directly. With upcoming changes to how `SnapshotInfo` is stored it would be nice to have better test coverage. This PR adds randomized coverage of serializing user metadata to a large number of tests that all user the shared infrastructure that is adjusted here.
|
Pinging @elastic/es-distributed (Team:Distributed) |
| throw new IllegalArgumentException("malformed metadata, should be an object"); | ||
| } | ||
| userMetadata((Map<String, Object>) entry.getValue()); | ||
| switch (name) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Admittedly sort of unrelated but it took me longer than it should have to track down how SLM configures its snapshots via this method and this change makes things at least a little easier to read IMO.
|
Jenkins run elasticsearch-ci/part-2 (unrelated watcher tests) |
fcofdez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
|
Thanks Francisco! |
|
relevant pieces from this one have been backported as part of #74676 |
We barely test the correct handling of user metadata directly.
With upcoming changes to how
SnapshotInfois stored it would be niceto have better test coverage. This PR adds randomized coverage of serializing
user metadata to a large number of tests that all user the shared infrastructure
that is adjusted here.