Skip to content

Commit

Permalink
Change the order of assertions to improve test stability
Browse files Browse the repository at this point in the history
This tests has been failing too much.

I tried to improve it by changing the order of the assertions. The
purpose of this test is to verify that committing already committed
offset results in 200.

But since it commits offset 25, which does not exist, given the topic
is empty, this may cause the stream to close depending if the commit
arrives before or after the stream code reaches a certain stage.

I kept all the conditions (testing for 200 and 204) but in a way that
we don't have to commit inexistent offset 25. I run it dozens of times
and it doesnt break anymore.
  • Loading branch information
rcillo committed Jul 17, 2020
1 parent 7f415ce commit 9551028
Showing 1 changed file with 7 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -249,25 +249,21 @@ public void testOffsetsCommit() throws Exception {
.start();
waitFor(() -> assertThat(client.getSessionId(), not(equalTo(SESSION_ID_UNKNOWN))));

String cursor = "{\"items\":[{\"partition\":\"0\",\"offset\":\"25\",\"event_type\":\"" + etName +
// commit lower offsets and expect 200
String cursor = "{\"items\":[{\"partition\":\"0\",\"offset\":\"-1\",\"event_type\":\"" + etName +
"\",\"cursor_token\":\"abc\"}]}";
commitCursors(subscription, cursor, client.getSessionId())
.then()
.statusCode(HttpStatus.SC_NO_CONTENT);

// check that offset is actually committed to Zookeeper
String committedOffset = getCommittedOffsetFromZk(etName, subscription, "0");
assertThat(committedOffset, equalTo(TestUtils.toTimelineOffset(25)));
.statusCode(HttpStatus.SC_OK);

// commit lower offsets and expect 200
cursor = "{\"items\":[{\"partition\":\"0\",\"offset\":\"10\",\"event_type\":\"" + etName +
cursor = "{\"items\":[{\"partition\":\"0\",\"offset\":\"25\",\"event_type\":\"" + etName +
"\",\"cursor_token\":\"abc\"}]}";
commitCursors(subscription, cursor, client.getSessionId())
.then()
.statusCode(HttpStatus.SC_OK);
.statusCode(HttpStatus.SC_NO_CONTENT);

// check that committed offset in Zookeeper is not changed
committedOffset = getCommittedOffsetFromZk(etName, subscription, "0");
// check that offset is actually committed to Zookeeper
String committedOffset = getCommittedOffsetFromZk(etName, subscription, "0");
assertThat(committedOffset, equalTo(TestUtils.toTimelineOffset(25)));
}

Expand Down

0 comments on commit 9551028

Please sign in to comment.