Skip to content

Conversation

danhermann
Copy link
Contributor

Fixes two problems with the testCopyFromOtherField test -- it should not choose a sub-object of the copy_from field as the target field and it should "deep-compare" any copied objects contained within maps as they will not be reference-equal after the fix in #69349.

Fixes #69876.

@danhermann danhermann added :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >bug v7.12.1 v7.13.0 v8.0.0 labels Mar 9, 2021
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Mar 9, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

Comment on lines +167 to +179
private static void assertMapEquals(Object actual, Object expected) {
if (expected instanceof Map) {
Map<?, ?> expectedMap = (Map<?, ?>) expected;
Map<?, ?> actualMap = (Map<?, ?>) actual;
assertThat(actualMap.keySet().toArray(), arrayContainingInAnyOrder(expectedMap.keySet().toArray()));
for (Map.Entry<?, ?> entry : actualMap.entrySet()) {
if (entry.getValue() instanceof Map) {
assertMapEquals(entry.getValue(), expectedMap.get(entry.getKey()));
} else {
assertThat(entry.getValue(), equalTo(expectedMap.get(entry.getKey())));
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would be happy if there were some Hamcrest magic that did this "deep-compare" of objects within maps automatically, but I couldn't find anything along those lines.

@danhermann
Copy link
Contributor Author

@elasticmachine update branch

@danhermann danhermann added >test Issues or PRs that are addressing/adding tests and removed >bug labels Mar 9, 2021
@martijnvg martijnvg requested a review from dakrone March 11, 2021 16:41
Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM

@danhermann
Copy link
Contributor Author

Thanks, @dakrone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests v7.12.1 v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failure in SetProcessorTests.testCopyFromOtherField
4 participants