Skip to content

Fix DissectParserTests expecting unique keys#39262

Merged
albertzaharovits merged 2 commits intoelastic:masterfrom
albertzaharovits:fix-dissect-parser-tests
Feb 22, 2019
Merged

Fix DissectParserTests expecting unique keys#39262
albertzaharovits merged 2 commits intoelastic:masterfrom
albertzaharovits:fix-dissect-parser-tests

Conversation

@albertzaharovits
Copy link
Copy Markdown
Contributor

@albertzaharovits albertzaharovits commented Feb 21, 2019

Fixes a bug in DissectParserTests where the tests expected dissect keys to be unique but were not.

Closes #39244

@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-features

for (int i = 0; i < results.size(); i++) {
final String key = expectedKeys.get(i);
assertThat(results.get(key), Matchers.equalTo(expectedValues.get(i)));
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This slightly changes the assertion by no longer explicitly asserting that that the keys match. Is this change needed to fix the test(s) ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It does check that the keys match, given that the keys are unique in the expectedKeys array. The original check was flawed because it was sorting parallel arrays independently.

Copy link
Copy Markdown
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

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

LGTM

@albertzaharovits albertzaharovits merged commit 1e7f28e into elastic:master Feb 22, 2019
@albertzaharovits albertzaharovits deleted the fix-dissect-parser-tests branch February 22, 2019 13:57
albertzaharovits added a commit that referenced this pull request Feb 22, 2019
Fixes a bug in DissectParserTests where the tests expected dissect
keys to be unique but were not.

Closes #39244
albertzaharovits added a commit that referenced this pull request Feb 22, 2019
Fixes a bug in DissectParserTests where the tests expected dissect
keys to be unique but were not.

Closes #39244
albertzaharovits added a commit that referenced this pull request Feb 22, 2019
Fixes a bug in DissectParserTests where the tests expected dissect
keys to be unique but were not.

Closes #39244
weizijun pushed a commit to weizijun/elasticsearch that referenced this pull request Feb 22, 2019
Fixes a bug in DissectParserTests where the tests expected dissect
keys to be unique but were not.

Closes elastic#39244
weizijun pushed a commit to weizijun/elasticsearch that referenced this pull request Feb 22, 2019
Fixes a bug in DissectParserTests where the tests expected dissect
keys to be unique but were not.

Closes elastic#39244
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>test Issues or PRs that are addressing/adding tests v6.7.0 v7.0.0-rc1 v7.2.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] DissectParserTests.testBasicMatchUnicode

4 participants