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

Compare event by event in testdata framework to avoid sorting problems #13747

Merged
merged 3 commits into from
Sep 23, 2019

Conversation

ChrsMark
Copy link
Member

In testdata we currently assert the final lists of events, which need to be sorted otherwise the assertion will fail. This leads to problematic situations when one want to use remove_fields_from_comparison in long lists of events since the sorting are not working properly.

In this regard, it might be safer in the long run to compare event by event the lists, as we do in

for _, event := range events {

@ChrsMark ChrsMark added enhancement Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Sep 20, 2019
@ChrsMark ChrsMark requested a review from jsoriano September 20, 2019 14:36
@ChrsMark ChrsMark self-assigned this Sep 20, 2019
@ChrsMark ChrsMark changed the title Compare event by event to avoid sorting problems Compare event by event in testdata framework to avoid sorting problems Sep 20, 2019
Signed-off-by: chrismark <[email protected]>
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Please move the changelog entry to the developer changelog before merging.

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
@ChrsMark
Copy link
Member Author

Failing test seems to be irrelevant (libbeat) and it is also failing for other PRs too. Merging since Metricbeat tests finished successfully.

@ChrsMark ChrsMark merged commit 3e0cf6c into elastic:master Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants