Skip to content
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

kbn-expect - add .eql support for sets #200034

Merged
merged 5 commits into from
Nov 14, 2024
Merged

Conversation

pheyos
Copy link
Member

@pheyos pheyos commented Nov 13, 2024

Summary

This PR adds support for asserting test equality with kbn-expect. It also introduces some unit tests to kbn-expect in order to verify the changes are working.

@pheyos pheyos self-assigned this Nov 13, 2024
@pheyos pheyos requested review from a team as code owners November 13, 2024 15:56
Comment on lines +650 to +653
// format sets like arrays
if (value instanceof Set) {
value = Array.from(value)
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The format doesn't know how to handle sets properly, so it applies the code for generic objects, which prints a failure message like this:

expected {} to sort of equal {}

The quickest way to get a proper output was treating the sets as arrays for formatting purposes, which prints out failure messages like this:

expected [ 'a', 'b', 'c' ] to sort of equal [ 'x', 'y', 'z' ]

@pheyos pheyos requested a review from dmlemeshko November 13, 2024 15:59
Copy link
Contributor

@fake-haris fake-haris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pheyos pheyos added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v9.0.0 labels Nov 13, 2024
Copy link
Member

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for adding tests.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

cc @pheyos

@pheyos pheyos merged commit 714382d into elastic:main Nov 14, 2024
22 checks passed
@pheyos pheyos deleted the fix_expect_for_sets branch November 14, 2024 13:59
wayneseymour pushed a commit to wayneseymour/kibana that referenced this pull request Nov 18, 2024
## Summary

This PR adds support for asserting test equality with kbn-expect. It
also introduces some unit tests to kbn-expect in order to verify the
changes are working.
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
## Summary

This PR adds support for asserting test equality with kbn-expect. It
also introduces some unit tests to kbn-expect in order to verify the
changes are working.
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
## Summary

This PR adds support for asserting test equality with kbn-expect. It
also introduces some unit tests to kbn-expect in order to verify the
changes are working.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants