diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2021-01-07 06:52:22 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-01-07 06:52:22 +0100 |
commit | 9263966a9c697f5acff53a595493e27a2f05e683 (patch) | |
tree | c6a85dc50a71b2fb0b6f21ccc5da3a3c80db7ae0 /documentapi/src/test/java/com | |
parent | 84d629c5125f2e0afc93ed730875215cb68f1f38 (diff) |
Revert "Keep reply with most severe error when merging error replies"
Diffstat (limited to 'documentapi/src/test/java/com')
2 files changed, 28 insertions, 23 deletions
diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/ReplyMergerTestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/ReplyMergerTestCase.java index 11d74800a79..157b4a6585b 100644 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/ReplyMergerTestCase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/ReplyMergerTestCase.java @@ -38,7 +38,7 @@ public class ReplyMergerTestCase { } @Test - public void mergingSingleReplyWithOneErrorReturnsSameReplyWithError() { + public void mergingSingleReplyWithOneErrorReturnsEmptyReplyWithError() { Reply r1 = new EmptyReply(); Error error = new Error(1234, "oh no!"); r1.addError(error); @@ -46,12 +46,12 @@ public class ReplyMergerTestCase { Tuple2<Integer, Reply> ret = merger.mergedReply(); assertNull(ret.first); - assertSame(r1, ret.second); + assertNotSame(r1, ret.second); assertThatErrorsMatch(new Error[] { error }, ret); } @Test - public void mergingSingleReplyWithMultipleErrorsReturnsSameReplyWithAllErrors() { + public void mergingSingleReplyWithMultipleErrorsReturnsEmptyReplyWithAllErrors() { Reply r1 = new EmptyReply(); Error errors[] = new Error[] { new Error(1234, "oh no!"), new Error(4567, "oh dear!"), @@ -62,12 +62,12 @@ public class ReplyMergerTestCase { Tuple2<Integer, Reply> ret = merger.mergedReply(); assertNull(ret.first); - assertSame(r1, ret.second); + assertNotSame(r1, ret.second); assertThatErrorsMatch(errors, ret); } @Test - public void mergingMultipleRepliesWithMultipleErrorsReturnsMostSevereReplyWithAllErrors() { + public void mergingMultipleRepliesWithMultipleErrorsReturnsEmptyReplyWithAllErrors() { Reply r1 = new EmptyReply(); Reply r2 = new EmptyReply(); Error errors[] = new Error[] { @@ -81,7 +81,7 @@ public class ReplyMergerTestCase { Tuple2<Integer, Reply> ret = merger.mergedReply(); assertNull(ret.first); - assertSame(r1, ret.second); + assertNotSame(r1, ret.second); assertNotSame(r2, ret.second); assertThatErrorsMatch(errors, ret); } @@ -143,7 +143,7 @@ public class ReplyMergerTestCase { merger.merge(1, r2); Tuple2<Integer, Reply> ret = merger.mergedReply(); assertNull(ret.first); - assertSame(r1, ret.second); + assertNotSame(r1, ret.second); assertNotSame(r2, ret.second); // All errors from replies with errors are included, not those that // are fully ignored. @@ -182,7 +182,7 @@ public class ReplyMergerTestCase { } @Test - // TODO jonmv: This seems wrong, and is probably a consequence of TAS being implemented after reply merging. + // TODO: This seems wrong, and is probably a consequence of TAS being added later than this logic was written. public void returnErrorDocumentReplyWhereDocWasFoundWhichIsProbablyWrong() { Error e1 = new Error(DocumentProtocol.ERROR_TEST_AND_SET_CONDITION_FAILED, "fail"); UpdateDocumentReply r1 = new UpdateDocumentReply(); @@ -197,7 +197,7 @@ public class ReplyMergerTestCase { merger.merge(2, r3); Tuple2<Integer, Reply> ret = merger.mergedReply(); assertNull(ret.first); - assertSame(r1, ret.second); + assertNotSame(r1, ret.second); assertThatErrorsMatch(new Error[] { e1 }, ret); } diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestFrame.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestFrame.java index 92e94256411..89d5db62899 100755 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestFrame.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestFrame.java @@ -155,6 +155,16 @@ public class PolicyTestFrame { } /** + * This is a convenience method for invoking {@link #assertMerge(Map,List,List)} with no expected value. + * + * @param replies The errors to set in the leaf node replies. + * @param expectedErrors The list of expected errors in the merged reply. + */ + public void assertMergeError(Map<String, Integer> replies, List<Integer> expectedErrors) { + assertMerge(replies, expectedErrors, null); + } + + /** * This is a convenience method for invoking {@link this#assertMerge(Map,List,List)} with no expected errors. * * @param replies The errors to set in the leaf node replies. @@ -223,10 +233,10 @@ public class PolicyTestFrame { Map<String, Integer> replies = new HashMap<>(); replies.put(recipient, ErrorCode.NONE); - assertMergeOk(replies, List.of(recipient)); + assertMergeOk(replies, Arrays.asList(recipient)); replies.put(recipient, ErrorCode.TRANSIENT_ERROR); - assertMerge(replies, List.of(ErrorCode.TRANSIENT_ERROR), List.of(recipient)); + assertMergeError(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR)); } /** @@ -242,33 +252,28 @@ public class PolicyTestFrame { Map<String, Integer> replies = new HashMap<>(); replies.put(recipientOne, ErrorCode.NONE); replies.put(recipientTwo, ErrorCode.NONE); - assertMergeOk(replies, List.of(recipientOne, recipientTwo)); + assertMergeOk(replies, Arrays.asList(recipientOne, recipientTwo)); replies.put(recipientOne, ErrorCode.TRANSIENT_ERROR); replies.put(recipientTwo, ErrorCode.NONE); - assertMerge(replies, List.of(ErrorCode.TRANSIENT_ERROR), List.of(recipientOne)); - - replies.put(recipientOne, ErrorCode.TRANSIENT_ERROR); - replies.put(recipientTwo, ErrorCode.FATAL_ERROR); - assertMerge(replies, List.of(ErrorCode.TRANSIENT_ERROR, ErrorCode.FATAL_ERROR), List.of(recipientTwo)); + assertMergeError(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR)); replies.put(recipientOne, ErrorCode.TRANSIENT_ERROR); replies.put(recipientTwo, ErrorCode.TRANSIENT_ERROR); - assertMerge(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR, ErrorCode.TRANSIENT_ERROR), List.of(recipientOne, recipientTwo)); + assertMergeError(replies, Arrays.asList(ErrorCode.TRANSIENT_ERROR, ErrorCode.TRANSIENT_ERROR)); replies.put(recipientOne, ErrorCode.NONE); replies.put(recipientTwo, DocumentProtocol.ERROR_MESSAGE_IGNORED); - assertMergeOk(replies, List.of(recipientOne)); + assertMergeOk(replies, Arrays.asList(recipientOne)); replies.put(recipientOne, DocumentProtocol.ERROR_MESSAGE_IGNORED); replies.put(recipientTwo, ErrorCode.NONE); - assertMergeOk(replies, List.of(recipientTwo)); + assertMergeOk(replies, Arrays.asList(recipientTwo)); replies.put(recipientOne, DocumentProtocol.ERROR_MESSAGE_IGNORED); replies.put(recipientTwo, DocumentProtocol.ERROR_MESSAGE_IGNORED); - assertMerge(replies, List.of(DocumentProtocol.ERROR_MESSAGE_IGNORED, - DocumentProtocol.ERROR_MESSAGE_IGNORED), - null); // Only ignored errors specifically causes an EmptyReply. + assertMergeError(replies, Arrays.asList(DocumentProtocol.ERROR_MESSAGE_IGNORED, + DocumentProtocol.ERROR_MESSAGE_IGNORED)); } /** |