From 2148bf294d662bc73a7724855ba22c3530e9ed22 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 6 Aug 2014 09:46:49 -0700 Subject: [PATCH] Acceptance tests: Always check error after SshSession.exec() The GitUtil.createProject() method in particular was failing silently in a surprising number of cases. Fix all of these, and add the error message to the AssertionError in cases where it was checked. Change-Id: Ibca7881edfe8be583a19cc08f2b942bf4a9bbfc3 --- .../com/google/gerrit/acceptance/GitUtil.java | 4 +++ .../gerrit/acceptance/git/SubmitOnPushIT.java | 32 ------------------ .../acceptance/git/VisibleRefFilterIT.java | 4 --- .../rest/group/DefaultGroupsIT.java | 2 ++ .../rest/project/ProjectLevelConfigIT.java | 33 ++----------------- .../gerrit/acceptance/ssh/BanCommitIT.java | 2 +- .../acceptance/ssh/GarbageCollectionIT.java | 4 +-- 7 files changed, 12 insertions(+), 69 deletions(-) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GitUtil.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GitUtil.java index 3234ffdf27..3371eb2ac0 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GitUtil.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/GitUtil.java @@ -100,6 +100,10 @@ public class GitUtil { b.append("\""); } s.exec(b.toString()); + if (s.hasError()) { + throw new IllegalStateException( + "gerrit create-project returned error: " + s.getError()); + } } public static Git cloneProject(String url) throws GitAPIException, IOException { diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java index d0f911b0a0..917c28340d 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java @@ -14,8 +14,6 @@ package com.google.gerrit.acceptance.git; -import static com.google.gerrit.acceptance.GitUtil.cloneProject; -import static com.google.gerrit.acceptance.GitUtil.createProject; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -24,7 +22,6 @@ import com.google.common.collect.Iterables; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.PushOneCommit; -import com.google.gerrit.acceptance.SshSession; import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.PermissionRule; @@ -33,7 +30,6 @@ import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.account.GroupCache; @@ -44,10 +40,8 @@ import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ProjectCache; import com.google.gwtorm.server.OrmException; -import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; -import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.RepositoryNotFoundException; @@ -57,18 +51,12 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.RefSpec; -import org.junit.After; -import org.junit.Before; import org.junit.Test; import java.io.IOException; @NoHttpd public class SubmitOnPushIT extends AbstractDaemonTest { - - @Inject - private SchemaFactory reviewDbProvider; - @Inject private GitRepositoryManager repoManager; @@ -93,26 +81,6 @@ public class SubmitOnPushIT extends AbstractDaemonTest { @Inject private PushOneCommit.Factory pushFactory; - private Project.NameKey project; - private Git git; - private ReviewDb db; - - @Before - public void setUp() throws Exception { - project = new Project.NameKey("p"); - SshSession sshSession = new SshSession(server, admin); - createProject(sshSession, project.get()); - git = cloneProject(sshSession.getUrl() + "/" + project.get()); - sshSession.close(); - - db = reviewDbProvider.open(); - } - - @After - public void cleanup() { - db.close(); - } - @Test public void submitOnPush() throws GitAPIException, OrmException, IOException, ConfigInvalidException { diff --git 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 index 82a0318109..4a1227cbed 100644 --- 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 @@ -14,7 +14,6 @@ package com.google.gerrit.acceptance.git; -import static com.google.gerrit.acceptance.GitUtil.createProject; import static com.google.gerrit.server.group.SystemGroupBackend.PROJECT_OWNERS; import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS; import static org.junit.Assert.assertEquals; @@ -27,7 +26,6 @@ import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.Permission; import com.google.gerrit.extensions.api.projects.BranchInput; -import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.project.Util; @@ -44,8 +42,6 @@ public class VisibleRefFilterIT extends AbstractDaemonTest { @Before public void setUp() throws Exception { - project = new Project.NameKey("p"); - createProject(sshSession, project.get()); setUpChanges(); setUpPermissions(); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/DefaultGroupsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/DefaultGroupsIT.java index 0d229ca529..f60b5dde29 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/DefaultGroupsIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/DefaultGroupsIT.java @@ -14,6 +14,7 @@ package com.google.gerrit.acceptance.rest.group; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import com.google.common.collect.Sets; @@ -47,6 +48,7 @@ public class DefaultGroupsIT extends AbstractDaemonTest { public void defaultGroupsCreated_ssh() throws JSchException, IOException { SshSession session = new SshSession(server, admin); String result = session.exec("gerrit ls-groups"); + assertFalse(session.getError(), session.hasError()); assertTrue(result.contains("Administrators")); assertTrue(result.contains("Non-Interactive Users")); session.close(); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java index fb830e4ec9..3b790b13ee 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ProjectLevelConfigIT.java @@ -16,36 +16,26 @@ package com.google.gerrit.acceptance.rest.project; import static com.google.gerrit.acceptance.GitUtil.checkout; import static com.google.gerrit.acceptance.GitUtil.cloneProject; -import static com.google.gerrit.acceptance.GitUtil.createProject; import static com.google.gerrit.acceptance.GitUtil.fetch; import static org.junit.Assert.assertEquals; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.PushOneCommit; -import com.google.gerrit.acceptance.SshSession; -import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.config.AllProjectsNameProvider; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectState; -import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.lib.Config; -import org.junit.After; import org.junit.Before; import org.junit.Test; import java.io.IOException; public class ProjectLevelConfigIT extends AbstractDaemonTest { - - @Inject - private SchemaFactory reviewDbProvider; - @Inject private ProjectCache projectCache; @@ -55,27 +45,10 @@ public class ProjectLevelConfigIT extends AbstractDaemonTest { @Inject private PushOneCommit.Factory pushFactory; - private ReviewDb db; - private SshSession sshSession; - private String project; - private Git git; - @Before public void setUp() throws Exception { - sshSession = new SshSession(server, admin); - - project = "p"; - createProject(sshSession, project, null, true); - git = cloneProject(sshSession.getUrl() + "/" + project); fetch(git, RefNames.REFS_CONFIG + ":refs/heads/config"); checkout(git, "refs/heads/config"); - - db = reviewDbProvider.open(); - } - - @After - public void cleanup() { - db.close(); } @Test @@ -89,13 +62,13 @@ public class ProjectLevelConfigIT extends AbstractDaemonTest { configName, cfg.toText()); push.to(git, RefNames.REFS_CONFIG); - ProjectState state = projectCache.get(new Project.NameKey(project)); + ProjectState state = projectCache.get(project); assertEquals(cfg.toText(), state.getConfig(configName).get().toText()); } @Test public void nonExistingConfig() { - ProjectState state = projectCache.get(new Project.NameKey(project)); + ProjectState state = projectCache.get(project); assertEquals("", state.getConfig("test.config").get().toText()); } @@ -125,7 +98,7 @@ public class ProjectLevelConfigIT extends AbstractDaemonTest { configName, cfg.toText()); push.to(git, RefNames.REFS_CONFIG); - ProjectState state = projectCache.get(new Project.NameKey(project)); + ProjectState state = projectCache.get(project); Config expectedCfg = new Config(); expectedCfg.setString("s1", null, "k1", "childValue1"); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java index bfce52304a..1bd1b71195 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java @@ -42,7 +42,7 @@ public class BanCommitIT extends AbstractDaemonTest { String response = sshSession.exec("gerrit ban-commit " + project.get() + " " + c.getCommit().getName()); - assertFalse(sshSession.hasError()); + assertFalse(sshSession.getError(), sshSession.hasError()); assertFalse(response, response.toLowerCase(Locale.US).contains("error")); PushResult pushResult = pushHead(git, "refs/heads/master", false); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/GarbageCollectionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/GarbageCollectionIT.java index 9f859dc723..d32a58f5b0 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/GarbageCollectionIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/GarbageCollectionIT.java @@ -76,7 +76,7 @@ public class GarbageCollectionIT extends AbstractDaemonTest { String response = sshSession.exec("gerrit gc \"" + project1.get() + "\" \"" + project2.get() + "\""); - assertFalse(sshSession.hasError()); + assertFalse(sshSession.getError(), sshSession.hasError()); assertNoError(response); gcAssert.assertHasPackFile(project1, project2); gcAssert.assertHasNoPackFile(allProjects, project3); @@ -86,7 +86,7 @@ public class GarbageCollectionIT extends AbstractDaemonTest { @UseLocalDisk public void testGcAll() throws JSchException, IOException { String response = sshSession.exec("gerrit gc --all"); - assertFalse(sshSession.hasError()); + assertFalse(sshSession.getError(), sshSession.hasError()); assertNoError(response); gcAssert.assertHasPackFile(allProjects, project1, project2, project3); }