opendev/gerrit
Code Issues Proposed changes

Remove query test for different timestamp resolutions

Browse Source 
We used to sort results based on the sortkey field in Change, which was
a combination of minute-resolution timestamp and change number. Thus it
made sense to have separate tests for timestamp resolutions above and
below 1 minute. However, sortkey was killed long ago, with the last
vestiges removed in 2015 (I87660f8e0), so it no longer serves a purpose
to have these separate methods.

One specific issue with these methods was the assertion in the
sub-minute resolution case that the updated timestamps of each change
differ by less than one minute. This assertion assumes that only Gerrit
is responsible for ticking the test clock, so we know there is some
bound on the number of ticks that happen in the test. But this is not a
safe assumption: TestTimeUtil mucks with global Joda state, and it's
possible that library code used in a secondary index implementation also
ticks the same clock. Thus there is no way to guarantee that this
assertion will pass.

Fortunately, for the reasons described above, this assertion wasn't
really serving any purpose, and it goes away in the reworked tests.

Change-Id: I7412b066ae6c98bcec788fba8c534c4a45f50a6f
This commit is contained in:
Dave Borowitz
2017-04-12 12:11:58 -04:00
parent 1f26b86cf4
commit 84eed99d92
1 changed files with 2 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -876,38 +876,14 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
}
@Test
public void updatedOrderWithMinuteResolution() throws Exception {
resetTimeWithClockStep(2, MINUTES);
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChange(repo);
Change change1 = insert(repo, ins1);
Change change2 = insert(repo, newChange(repo));
assertThat(lastUpdatedMs(change1)).isLessThan(lastUpdatedMs(change2));
assertQuery("status:new", change2, change1);
gApi.changes().id(change1.getId().get()).topic("new-topic");
change1 = notesFactory.create(db, change1.getProject(), change1.getId()).getChange();
assertThat(lastUpdatedMs(change1)).isGreaterThan(lastUpdatedMs(change2));
assertThat(lastUpdatedMs(change1) - lastUpdatedMs(change2))
.isGreaterThan(MILLISECONDS.convert(1, MINUTES));
// change1 moved to the top.
assertQuery("status:new", change1, change2);
}
@Test
public void updatedOrderWithSubMinuteResolution() throws Exception {
public void updatedOrder() throws Exception {
resetTimeWithClockStep(1, SECONDS);
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins1 = newChange(repo);
Change change1 = insert(repo, ins1);
Change change2 = insert(repo, newChange(repo));
assertThat(lastUpdatedMs(change1)).isLessThan(lastUpdatedMs(change2));
assertQuery("status:new", change2, change1);
gApi.changes().id(change1.getId().get()).topic("new-topic");
@@ -915,7 +891,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
assertThat(lastUpdatedMs(change1)).isGreaterThan(lastUpdatedMs(change2));
assertThat(lastUpdatedMs(change1) - lastUpdatedMs(change2))
.isLessThan(MILLISECONDS.convert(1, MINUTES));
.isAtLeast(MILLISECONDS.convert(1, SECONDS));
// change1 moved to the top.
assertQuery("status:new", change1, change2);