Acceptance tests: Don't assume counters start at 1

Instead of hard-coding change IDs, cache hit counts, etc., record the actual values and use them. Similarly, scope search results to projects where appropriate so that total change counts are correct. Change-Id: I2d0acc17eb186d4c4de364fe1a9e06e1b4dba777
2015-03-27 09:37:03 -07:00
parent 9ed5325b0c
commit 1a51cd2457
4 changed files with 74 additions and 56 deletions
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -33,11 +33,13 @@ import com.google.gerrit.extensions.common.LabelInfo;
 import com.google.gerrit.extensions.common.RevisionInfo;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;

 import org.eclipse.jgit.lib.Constants;
 import org.junit.Test;

+import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Set;
@@ -58,7 +60,7 @@ public class ChangeIT extends AbstractDaemonTest {
    assertThat(c.mergeable).isTrue();
    assertThat(c.changeId).isEqualTo(r.getChangeId());
    assertThat(c.created).isEqualTo(c.updated);
-    assertThat(c._number).is(1);
+    assertThat(c._number).is(r.getChange().getId().get());

    assertThat(c.owner._accountId).is(admin.getId().get());
    assertThat(c.owner.name).isNull();
@@ -233,38 +235,35 @@ public class ChangeIT extends AbstractDaemonTest {

  @Test
  public void queryChangesNoQuery() throws Exception {
-    PushOneCommit.Result r1 = createChange();
-    PushOneCommit.Result r2 = createChange();
+    PushOneCommit.Result r = createChange();
    List<ChangeInfo> results = gApi.changes().query().get();
-    assertThat(results).hasSize(2);
-    assertThat(results.get(0).changeId).isEqualTo(r2.getChangeId());
-    assertThat(results.get(1).changeId).isEqualTo(r1.getChangeId());
+    assertThat(results.size()).isAtLeast(1);
+    List<Integer> ids = new ArrayList<>(results.size());
+    for (int i = 0; i < results.size(); i++) {
+      ChangeInfo info = results.get(i);
+      if (i == 0) {
+        assertThat(info._number).isEqualTo(r.getChange().getId().get());
+      }
+      assertThat(Change.Status.forChangeStatus(info.status).isOpen()).isTrue();
+      ids.add(info._number);
+    }
+    assertThat(ids).contains(r.getChange().getId().get());
  }

  @Test
  public void queryChangesNoResults() throws Exception {
    createChange();
-    List<ChangeInfo> results = query("status:open");
-    assertThat(results).hasSize(1);
-    results = query("status:closed");
-    assertThat(results).isEmpty();
+    assertThat(query("message:test")).isNotEmpty();
+    assertThat(query("message:{" + getClass().getName() + "fhqwhgads}"))
+        .isEmpty();
  }

  @Test
-  public void queryChangesOneTerm() throws Exception {
-    PushOneCommit.Result r1 = createChange();
-    PushOneCommit.Result r2 = createChange();
-    List<ChangeInfo> results = query("status:open");
-    assertThat(results).hasSize(2);
-    assertThat(results.get(0).changeId).isEqualTo(r2.getChangeId());
-    assertThat(results.get(1).changeId).isEqualTo(r1.getChangeId());
-  }
-
-  @Test
-  public void queryChangesMultipleTerms() throws Exception {
+  public void queryChanges() throws Exception {
    PushOneCommit.Result r1 = createChange();
    createChange();
-    List<ChangeInfo> results = query("status:open " + r1.getChangeId());
+    List<ChangeInfo> results =
+        query("project:{" + project.get() + "} " + r1.getChangeId());
    assertThat(Iterables.getOnlyElement(results).changeId)
        .isEqualTo(r1.getChangeId());
  }
@@ -283,7 +282,8 @@ public class ChangeIT extends AbstractDaemonTest {
  public void queryChangesStart() throws Exception {
    PushOneCommit.Result r1 = createChange();
    createChange();
-    List<ChangeInfo> results = gApi.changes().query().withStart(1).get();
+    List<ChangeInfo> results = gApi.changes()
+        .query("project:{" + project.get() + "}").withStart(1).get();
    assertThat(Iterables.getOnlyElement(results).changeId)
        .isEqualTo(r1.getChangeId());
  }
@@ -321,7 +321,8 @@ public class ChangeIT extends AbstractDaemonTest {
  @Test
  public void queryChangesOwnerWithDifferentUsers() throws Exception {
    PushOneCommit.Result r = createChange();
-    assertThat(Iterables.getOnlyElement(query("owner:self")).changeId)
+    assertThat(Iterables.getOnlyElement(
+            query("project:{" + project.get() + "} owner:self")).changeId)
        .isEqualTo(r.getChangeId());
    setApiUser(user);
    assertThat(query("owner:self")).isEmpty();
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/VisibleRefFilterIT.java
@@ -68,22 +68,35 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {

  private AccountGroup.UUID admins;

+  private Change.Id c1;
+  private Change.Id c2;
+  private String r1;
+  private String r2;
+
  @Before
  public void setUp() throws Exception {
    admins = groupCache.get(new AccountGroup.NameKey("Administrators"))
        .getGroupUUID();
-    setUpChanges();
    setUpPermissions();
+    setUpChanges();
  }

  private void setUpPermissions() throws Exception {
+    // Remove read permissions for all users besides admin. This method is
+    // idempotent, so is safe to call on every test setup.
    ProjectConfig pc = projectCache.checkedGet(allProjects).getConfig();
    for (AccessSection sec : pc.getAccessSections()) {
      sec.removePermission(Permission.READ);
    }
+    Util.allow(pc, Permission.READ, admins, "refs/*");
    saveProjectConfig(allProjects, pc);
  }

+  private static String changeRefPrefix(Change.Id id) {
+    String ps = new PatchSet.Id(id, 1).toRefName();
+    return ps.substring(0, ps.length() - 1);
+  }
+
  private void setUpChanges() throws Exception {
    gApi.projects()
        .name(project.get())
@@ -94,9 +107,13 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
    PushOneCommit.Result mr = pushFactory.create(db, admin.getIdent(), testRepo)
        .to("refs/for/master%submit");
    mr.assertOkStatus();
+    c1 = mr.getChange().getId();
+    r1 = changeRefPrefix(c1);
    PushOneCommit.Result br = pushFactory.create(db, admin.getIdent(), testRepo)
        .to("refs/for/branch%submit");
    br.assertOkStatus();
+    c2 = br.getChange().getId();
+    r2 = changeRefPrefix(c2);

    Repository repo = repoManager.openRepository(project);
    try {
@@ -126,10 +143,10 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {

    assertRefs(
        "HEAD",
-        "refs/changes/01/1/1",
-        "refs/changes/01/1/meta",
-        "refs/changes/02/2/1",
-        "refs/changes/02/2/meta",
+        r1 + "1",
+        r1 + "meta",
+        r2 + "1",
+        r2 + "meta",
        "refs/heads/branch",
        "refs/heads/master",
        "refs/tags/branch-tag",
@@ -143,10 +160,10 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {

    assertRefs(
        "HEAD",
-        "refs/changes/01/1/1",
-        "refs/changes/01/1/meta",
-        "refs/changes/02/2/1",
-        "refs/changes/02/2/meta",
+        r1 + "1",
+        r1 + "meta",
+        r2 + "1",
+        r2 + "meta",
        "refs/heads/branch",
        "refs/heads/master",
        "refs/meta/config",
@@ -161,8 +178,8 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {

    assertRefs(
        "HEAD",
-        "refs/changes/01/1/1",
-        "refs/changes/01/1/meta",
+        r1 + "1",
+        r1 + "meta",
        "refs/heads/master",
        "refs/tags/master-tag");
  }
@@ -173,8 +190,8 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
    allow(Permission.READ, REGISTERED_USERS, "refs/heads/branch");

    assertRefs(
-        "refs/changes/02/2/1",
-        "refs/changes/02/2/meta",
+        r2 + "1",
+        r2 + "meta",
        "refs/heads/branch",
        "refs/tags/branch-tag",
        // master branch is not visible but master-tag is reachable from branch
@@ -187,24 +204,24 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
    allow(Permission.READ, REGISTERED_USERS, "refs/heads/master");
    deny(Permission.READ, REGISTERED_USERS, "refs/heads/branch");

-    Change c1 = db.changes().get(new Change.Id(1));
-    PatchSet ps1 = db.patchSets().get(new PatchSet.Id(c1.getId(), 1));
+    Change change1 = db.changes().get(c1);
+    PatchSet ps1 = db.patchSets().get(new PatchSet.Id(c1, 1));

    // Admin's edit is not visible.
    setApiUser(admin);
-    editModifier.createEdit(c1, ps1);
+    editModifier.createEdit(change1, ps1);

    // User's edit is visible.
    setApiUser(user);
-    editModifier.createEdit(c1, ps1);
+    editModifier.createEdit(change1, ps1);

    assertRefs(
        "HEAD",
-        "refs/changes/01/1/1",
-        "refs/changes/01/1/meta",
+        r1 + "1",
+        r1 + "meta",
        "refs/heads/master",
        "refs/tags/master-tag",
-        "refs/users/01/1000001/edit-1/1");
+        "refs/users/01/1000001/edit-" + c1.get() + "/1");
  }

  @Test
@@ -214,27 +231,27 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
      deny(Permission.READ, REGISTERED_USERS, "refs/heads/master");
      allow(Permission.READ, REGISTERED_USERS, "refs/heads/branch");

-      Change c1 = db.changes().get(new Change.Id(1));
-      PatchSet ps1 = db.patchSets().get(new PatchSet.Id(c1.getId(), 1));
+      Change change1 = db.changes().get(c1);
+      PatchSet ps1 = db.patchSets().get(new PatchSet.Id(c1, 1));
      setApiUser(admin);
-      editModifier.createEdit(c1, ps1);
+      editModifier.createEdit(change1, ps1);
      setApiUser(user);
-      editModifier.createEdit(c1, ps1);
+      editModifier.createEdit(change1, ps1);

      assertRefs(
          // Change 1 is visible due to accessDatabase capability, even though
          // refs/heads/master is not.
-          "refs/changes/01/1/1",
-          "refs/changes/01/1/meta",
-          "refs/changes/02/2/1",
-          "refs/changes/02/2/meta",
+          r1 + "1",
+          r1 + "meta",
+          r2 + "1",
+          r2 + "meta",
          "refs/heads/branch",
          "refs/tags/branch-tag",
          // See comment in subsetOfBranchesVisibleNotIncludingHead.
          "refs/tags/master-tag",
          // All edits are visible due to accessDatabase capability.
-          "refs/users/00/1000000/edit-1/1",
-          "refs/users/01/1000001/edit-1/1");
+          "refs/users/00/1000000/edit-" + c1.get() + "/1",
+          "refs/users/01/1000001/edit-" + c1.get() + "/1");
    } finally {
      removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
    }
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/GetCacheIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/GetCacheIT.java
@@ -34,7 +34,7 @@ public class GetCacheIT extends AbstractDaemonTest {

    assertThat(result.name).isEqualTo("accounts");
    assertThat(result.type).isEqualTo(CacheType.MEM);
-    assertThat(result.entries.mem).isEqualTo(1);
+    assertThat(result.entries.mem).isAtLeast(1L);
    assertThat(result.averageGet).isNotNull();
    assertThat(result.averageGet).endsWith("s");
    assertThat(result.entries.disk).isNull();
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/ListCachesIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/ListCachesIT.java
@@ -45,7 +45,7 @@ public class ListCachesIT extends AbstractDaemonTest {
    assertThat(result).containsKey("accounts");
    CacheInfo accountsCacheInfo = result.get("accounts");
    assertThat(accountsCacheInfo.type).isEqualTo(CacheType.MEM);
-    assertThat(accountsCacheInfo.entries.mem).isEqualTo(1);
+    assertThat(accountsCacheInfo.entries.mem).isAtLeast(1L);
    assertThat(accountsCacheInfo.averageGet).isNotNull();
    assertThat(accountsCacheInfo.averageGet).endsWith("s");
    assertThat(accountsCacheInfo.entries.disk).isNull();