From 694fd8896d2133fd5183349278be37e4f0c53d58 Mon Sep 17 00:00:00 2001 From: chirontt Date: Sun, 28 Apr 2019 15:51:21 -0400 Subject: [PATCH] Fix for various test failures. Most of failures were due to temporary test repos, users and/or teams being left behind after the test run, and these left-over stuff in $baseFolder/data/git caused assertion errors in many tests in subsequent test runs. This fix tries to delete those left-over stuff at the end of each test, mainly in their @Afterclass code blocks. PushLogTest.java is deleted as it doesn't work, and has been superseded with better tests in various protocol test suites (GitServletTest, GitDaemonTest, SshDaemonTest, etc.) --- .../com/gitblit/tests/FederationTests.java | 52 +++++++++++++++---- .../com/gitblit/tests/GitServletTest.java | 19 ++++--- .../tests/HtpasswdAuthenticationTest.java | 4 +- .../java/com/gitblit/tests/JsonUtilsTest.java | 9 ++-- .../java/com/gitblit/tests/PushLogTest.java | 41 --------------- src/test/java/com/gitblit/tests/RpcTests.java | 15 +++++- .../gitblit/tests/TicketReferenceTest.java | 21 +++++++- .../com/gitblit/tests/TicketServiceTest.java | 6 ++- .../java/com/gitblit/tests/UITicketTest.java | 35 ++++++------- 9 files changed, 112 insertions(+), 90 deletions(-) delete mode 100644 src/test/java/com/gitblit/tests/PushLogTest.java diff --git a/src/test/java/com/gitblit/tests/FederationTests.java b/src/test/java/com/gitblit/tests/FederationTests.java index 144fe3da..36ea56c3 100644 --- a/src/test/java/com/gitblit/tests/FederationTests.java +++ b/src/test/java/com/gitblit/tests/FederationTests.java @@ -15,6 +15,7 @@ */ package com.gitblit.tests; +import java.io.IOException; import java.util.Date; import java.util.HashMap; import java.util.List; @@ -45,6 +46,12 @@ public class FederationTests extends GitblitUnitTest { String password = GitBlitSuite.password; String token = "d7cc58921a80b37e0329a4dae2f9af38bf61ef5c"; + //test data + static final String testUser = "test"; + static final String testUserPwd = "whocares"; + static final String testTeam = "testteam"; + static final String testTeamRepository = "helloworld.git"; + private static final AtomicBoolean started = new AtomicBoolean(false); @BeforeClass @@ -54,11 +61,27 @@ public class FederationTests extends GitblitUnitTest { @AfterClass public static void stopGitblit() throws Exception { + //clean up test user and team if left over + deleteTestUser(); + deleteTestTeam(); + if (started.get()) { GitBlitSuite.stopGitblit(); } } + private static void deleteTestUser() throws IOException { + UserModel user = new UserModel(testUser); + user.password = testUserPwd; + RpcUtils.deleteUser(user, GitBlitSuite.url, GitBlitSuite.account, GitBlitSuite.password.toCharArray()); + } + + private static void deleteTestTeam() throws IOException { + TeamModel team = new TeamModel(testTeam); + team.addRepositoryPermission(testTeamRepository); + RpcUtils.deleteTeam(team, GitBlitSuite.url, GitBlitSuite.account, GitBlitSuite.password.toCharArray()); + } + @Test public void testProposal() throws Exception { // create dummy repository data @@ -121,18 +144,22 @@ public class FederationTests extends GitblitUnitTest { @Test public void testPullUsers() throws Exception { + //clean up test user and team left over from previous run, if any + deleteTestUser(); + deleteTestTeam(); + List users = FederationUtils.getUsers(getRegistration()); assertNotNull(users); - // admin is excluded - assertEquals(0, users.size()); + // admin is excluded, hence there should be no other users in the list + assertEquals("Gitblit server still contains " + users + " user account(s).", 0, users.size()); - UserModel newUser = new UserModel("test"); - newUser.password = "whocares"; + UserModel newUser = new UserModel(testUser); + newUser.password = testUserPwd; assertTrue(RpcUtils.createUser(newUser, url, account, password.toCharArray())); - TeamModel team = new TeamModel("testteam"); - team.addUser("test"); - team.addRepositoryPermission("helloworld.git"); + TeamModel team = new TeamModel(testTeam); + team.addUser(testUser); + team.addRepositoryPermission(testTeamRepository); assertTrue(RpcUtils.createTeam(team, url, account, password.toCharArray())); users = FederationUtils.getUsers(getRegistration()); @@ -140,7 +167,7 @@ public class FederationTests extends GitblitUnitTest { assertEquals(1, users.size()); newUser = users.get(0); - assertTrue(newUser.isTeamMember("testteam")); + assertTrue(newUser.isTeamMember(testTeam)); assertTrue(RpcUtils.deleteUser(newUser, url, account, password.toCharArray())); assertTrue(RpcUtils.deleteTeam(team, url, account, password.toCharArray())); @@ -148,9 +175,12 @@ public class FederationTests extends GitblitUnitTest { @Test public void testPullTeams() throws Exception { - TeamModel team = new TeamModel("testteam"); - team.addUser("test"); - team.addRepositoryPermission("helloworld.git"); + //clean up test team left over from previous run, if any + deleteTestTeam(); + + TeamModel team = new TeamModel(testTeam); + team.addUser(testUser); + team.addRepositoryPermission(testTeamRepository); assertTrue(RpcUtils.createTeam(team, url, account, password.toCharArray())); List teams = FederationUtils.getTeams(getRegistration()); diff --git a/src/test/java/com/gitblit/tests/GitServletTest.java b/src/test/java/com/gitblit/tests/GitServletTest.java index 705684a3..f9a1bec4 100644 --- a/src/test/java/com/gitblit/tests/GitServletTest.java +++ b/src/test/java/com/gitblit/tests/GitServletTest.java @@ -89,6 +89,9 @@ public class GitServletTest extends GitblitUnitTest { @BeforeClass public static void startGitblit() throws Exception { + //"refchecks" folder is used in this test class; + //need be deleted before Gitblit server instance is started + GitBlitSuite.deleteRefChecksFolder(); started.set(GitBlitSuite.startGitblit()); delete(getUser()); @@ -107,19 +110,19 @@ public class GitServletTest extends GitblitUnitTest { public static void deleteWorkingFolders() throws Exception { if (ticgitFolder.exists()) { GitBlitSuite.close(ticgitFolder); - FileUtils.delete(ticgitFolder, FileUtils.RECURSIVE); + FileUtils.delete(ticgitFolder, FileUtils.RECURSIVE | FileUtils.RETRY); } if (ticgit2Folder.exists()) { GitBlitSuite.close(ticgit2Folder); - FileUtils.delete(ticgit2Folder, FileUtils.RECURSIVE); + FileUtils.delete(ticgit2Folder, FileUtils.RECURSIVE | FileUtils.RETRY); } if (jgitFolder.exists()) { GitBlitSuite.close(jgitFolder); - FileUtils.delete(jgitFolder, FileUtils.RECURSIVE); + FileUtils.delete(jgitFolder, FileUtils.RECURSIVE | FileUtils.RETRY); } if (jgit2Folder.exists()) { GitBlitSuite.close(jgit2Folder); - FileUtils.delete(jgit2Folder, FileUtils.RECURSIVE); + FileUtils.delete(jgit2Folder, FileUtils.RECURSIVE | FileUtils.RETRY); } } @@ -400,7 +403,7 @@ public class GitServletTest extends GitblitUnitTest { // fork from original to a temporary bare repo File verification = new File(GitBlitSuite.REPOSITORIES, "refchecks/verify-committer.git"); if (verification.exists()) { - FileUtils.delete(verification, FileUtils.RECURSIVE); + FileUtils.delete(verification, FileUtils.RECURSIVE | FileUtils.RETRY); } CloneCommand clone = Git.cloneRepository(); clone.setURI(MessageFormat.format("{0}/ticgit.git", url)); @@ -485,7 +488,7 @@ public class GitServletTest extends GitblitUnitTest { // fork from original to a temporary bare repo File verification = new File(GitBlitSuite.REPOSITORIES, "refchecks/verify-committer.git"); if (verification.exists()) { - FileUtils.delete(verification, FileUtils.RECURSIVE); + FileUtils.delete(verification, FileUtils.RECURSIVE | FileUtils.RETRY); } CloneCommand clone = Git.cloneRepository(); clone.setURI(MessageFormat.format("{0}/ticgit.git", url)); @@ -630,7 +633,7 @@ public class GitServletTest extends GitblitUnitTest { // fork from original to a temporary bare repo File refChecks = new File(GitBlitSuite.REPOSITORIES, forkName); if (refChecks.exists()) { - FileUtils.delete(refChecks, FileUtils.RECURSIVE); + FileUtils.delete(refChecks, FileUtils.RECURSIVE | FileUtils.RETRY); } CloneCommand clone = Git.cloneRepository(); clone.setURI(url + "/" + originName); @@ -663,7 +666,7 @@ public class GitServletTest extends GitblitUnitTest { // clone temp bare repo to working copy File local = new File(GitBlitSuite.REPOSITORIES, workingCopy); if (local.exists()) { - FileUtils.delete(local, FileUtils.RECURSIVE); + FileUtils.delete(local, FileUtils.RECURSIVE | FileUtils.RETRY); } clone = Git.cloneRepository(); clone.setURI(MessageFormat.format("{0}/{1}", url, model.name)); diff --git a/src/test/java/com/gitblit/tests/HtpasswdAuthenticationTest.java b/src/test/java/com/gitblit/tests/HtpasswdAuthenticationTest.java index 26a49b24..40379f7d 100644 --- a/src/test/java/com/gitblit/tests/HtpasswdAuthenticationTest.java +++ b/src/test/java/com/gitblit/tests/HtpasswdAuthenticationTest.java @@ -200,11 +200,11 @@ public class HtpasswdAuthenticationTest extends GitblitUnitTest { public void testAuthenticationManager() { MS.put(KEY_SUPPORT_PLAINTEXT_PWD, "true"); - UserModel user = auth.authenticate("user1", "pass1".toCharArray(), null); + UserModel user = auth.authenticate("user1", "#externalAccount".toCharArray(), null); assertNotNull(user); assertEquals("user1", user.username); - user = auth.authenticate("user2", "pass2".toCharArray(), null); + user = auth.authenticate("user2", "#externalAccount".toCharArray(), null); assertNotNull(user); assertEquals("user2", user.username); diff --git a/src/test/java/com/gitblit/tests/JsonUtilsTest.java b/src/test/java/com/gitblit/tests/JsonUtilsTest.java index 1cd31b8b..b9cfc7a6 100644 --- a/src/test/java/com/gitblit/tests/JsonUtilsTest.java +++ b/src/test/java/com/gitblit/tests/JsonUtilsTest.java @@ -17,7 +17,7 @@ package com.gitblit.tests; import java.text.SimpleDateFormat; import java.util.Date; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import org.junit.Test; @@ -29,7 +29,8 @@ public class JsonUtilsTest extends GitblitUnitTest { @Test public void testSerialization() { - Map map = new HashMap(); + Map map = new LinkedHashMap(); + //LinkedHashMap preserves the order of insertion map.put("a", "alligator"); map.put("b", "bear"); map.put("c", "caterpillar"); @@ -37,7 +38,7 @@ public class JsonUtilsTest extends GitblitUnitTest { map.put("e", "eagle"); String json = JsonUtils.toJsonString(map); assertEquals( - "{\"d\":\"dingo\",\"e\":\"eagle\",\"b\":\"bear\",\"c\":\"caterpillar\",\"a\":\"alligator\"}", + "{\"a\":\"alligator\",\"b\":\"bear\",\"c\":\"caterpillar\",\"d\":\"dingo\",\"e\":\"eagle\"}", json); Map map2 = JsonUtils.fromJsonString(json, new TypeToken>() { @@ -56,4 +57,4 @@ public class JsonUtilsTest extends GitblitUnitTest { Date date = new Date(); String name = "myJson"; } -} \ No newline at end of file +} diff --git a/src/test/java/com/gitblit/tests/PushLogTest.java b/src/test/java/com/gitblit/tests/PushLogTest.java deleted file mode 100644 index be097cc4..00000000 --- a/src/test/java/com/gitblit/tests/PushLogTest.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2013 gitblit.com. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.gitblit.tests; - -import java.io.File; -import java.io.IOException; -import java.util.List; - -import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.RepositoryCache.FileKey; -import org.eclipse.jgit.storage.file.FileRepositoryBuilder; -import org.eclipse.jgit.util.FS; -import org.junit.Test; - -import com.gitblit.models.RefLogEntry; -import com.gitblit.utils.RefLogUtils; - -public class PushLogTest extends GitblitUnitTest { - - @Test - public void testPushLog() throws IOException { - String name = "~james/helloworld.git"; - File gitDir = FileKey.resolve(new File(GitBlitSuite.REPOSITORIES, name), FS.DETECTED); - Repository repository = new FileRepositoryBuilder().setGitDir(gitDir).build(); - List pushes = RefLogUtils.getRefLog(name, repository); - GitBlitSuite.close(repository); - } -} \ No newline at end of file diff --git a/src/test/java/com/gitblit/tests/RpcTests.java b/src/test/java/com/gitblit/tests/RpcTests.java index 51b4671b..4c794656 100644 --- a/src/test/java/com/gitblit/tests/RpcTests.java +++ b/src/test/java/com/gitblit/tests/RpcTests.java @@ -68,6 +68,11 @@ public class RpcTests extends GitblitUnitTest { @AfterClass public static void stopGitblit() throws Exception { + //clean up the "A-Team" if left over + TeamModel aTeam = new TeamModel("A-Team"); + aTeam.addRepositoryPermission("helloworld.git"); + RpcUtils.deleteTeam(aTeam, GitBlitSuite.url, GitBlitSuite.account, GitBlitSuite.password.toCharArray()); + if (started.get()) { GitBlitSuite.stopGitblit(); } @@ -265,11 +270,17 @@ public class RpcTests extends GitblitUnitTest { @Test public void testTeamAdministration() throws IOException { + //clean up the "A-Team" left over from previous run, if any + TeamModel aTeam = new TeamModel("A-Team"); + aTeam.addRepositoryPermission("helloworld.git"); + RpcUtils.deleteTeam(aTeam, url, account, password.toCharArray()); + List teams = RpcUtils.getTeams(url, account, password.toCharArray()); - assertEquals(1, teams.size()); + //should be just the admins team + assertEquals("In addition to 'admins', too many left-over team(s) in Gitblit server: " + teams, 1, teams.size()); // Create the A-Team - TeamModel aTeam = new TeamModel("A-Team"); + aTeam = new TeamModel("A-Team"); aTeam.users.add("admin"); aTeam.addRepositoryPermission("helloworld.git"); assertTrue(RpcUtils.createTeam(aTeam, url, account, password.toCharArray())); diff --git a/src/test/java/com/gitblit/tests/TicketReferenceTest.java b/src/test/java/com/gitblit/tests/TicketReferenceTest.java index 59e00c53..827ba59d 100644 --- a/src/test/java/com/gitblit/tests/TicketReferenceTest.java +++ b/src/test/java/com/gitblit/tests/TicketReferenceTest.java @@ -57,6 +57,7 @@ import com.gitblit.tickets.ITicketService; */ public class TicketReferenceTest extends GitblitUnitTest { + static final String repoName = "TicketReferenceTest.git"; static File workingCopy = new File(GitBlitSuite.REPOSITORIES, "working/TicketReferenceTest.git-wc"); static ITicketService ticketService; @@ -73,13 +74,13 @@ public class TicketReferenceTest extends GitblitUnitTest { @BeforeClass public static void configure() throws Exception { - File repositoryName = new File("TicketReferenceTest.git");; + File repositoryName = new File(repoName); GitBlitSuite.close(repositoryName); if (repositoryName.exists()) { FileUtils.delete(repositoryName, FileUtils.RECURSIVE | FileUtils.RETRY); } - repo = new RepositoryModel("TicketReferenceTest.git", null, null, null); + repo = new RepositoryModel(repoName, null, null, null); if (gitblit().hasRepository(repo.name)) { gitblit().deleteRepositoryModel(repo); @@ -141,7 +142,23 @@ public class TicketReferenceTest extends GitblitUnitTest { @AfterClass public static void cleanup() throws Exception { + //clean up the test user account if left over + if (gitblit().getUserModel(user.username) != null) { + gitblit().deleteUser(user.username); + } + GitBlitSuite.close(git); + + //clean up the TicketReferenceTest.git repo + File repositoryName = new File(repoName); + GitBlitSuite.close(repositoryName); + if (repositoryName.exists()) { + FileUtils.delete(repositoryName, FileUtils.RECURSIVE | FileUtils.RETRY); + } + RepositoryModel repo = new RepositoryModel(repoName, null, null, null); + if (gitblit().hasRepository(repo.name)) { + gitblit().deleteRepositoryModel(repo); + } } @Test diff --git a/src/test/java/com/gitblit/tests/TicketServiceTest.java b/src/test/java/com/gitblit/tests/TicketServiceTest.java index c654383d..c0d93531 100644 --- a/src/test/java/com/gitblit/tests/TicketServiceTest.java +++ b/src/test/java/com/gitblit/tests/TicketServiceTest.java @@ -21,8 +21,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import org.apache.commons.io.FileUtils; import org.bouncycastle.util.Arrays; +import org.eclipse.jgit.util.FileUtils; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -65,7 +65,9 @@ public abstract class TicketServiceTest extends GitblitUnitTest { protected IStoredSettings getSettings(boolean deleteAll) throws Exception { File dir = new File(GitBlitSuite.REPOSITORIES, getRepository().name); if (deleteAll) { - FileUtils.deleteDirectory(dir); + if (dir.exists()) { + FileUtils.delete(dir, FileUtils.RECURSIVE | FileUtils.RETRY); + } JGitUtils.createRepository(GitBlitSuite.REPOSITORIES, getRepository().name).close(); } diff --git a/src/test/java/com/gitblit/tests/UITicketTest.java b/src/test/java/com/gitblit/tests/UITicketTest.java index e89c32fc..266bee40 100644 --- a/src/test/java/com/gitblit/tests/UITicketTest.java +++ b/src/test/java/com/gitblit/tests/UITicketTest.java @@ -16,15 +16,13 @@ package com.gitblit.tests; import java.io.File; -import java.util.Date; +import java.io.IOException; import java.util.HashMap; -import java.util.List; import java.util.Map; -import org.apache.commons.io.FileUtils; -import org.bouncycastle.util.Arrays; -import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.util.FileUtils; import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; import org.junit.Test; @@ -40,26 +38,16 @@ import com.gitblit.manager.PluginManager; import com.gitblit.manager.RepositoryManager; import com.gitblit.manager.RuntimeManager; import com.gitblit.manager.UserManager; -import com.gitblit.models.Mailing; import com.gitblit.models.RepositoryModel; import com.gitblit.models.TicketModel; -import com.gitblit.models.TicketModel.Attachment; import com.gitblit.models.TicketModel.Change; import com.gitblit.models.TicketModel.Field; -import com.gitblit.models.TicketModel.Patchset; import com.gitblit.models.TicketModel.Priority; import com.gitblit.models.TicketModel.Severity; -import com.gitblit.models.TicketModel.Status; import com.gitblit.models.TicketModel.Type; import com.gitblit.tests.mock.MemorySettings; import com.gitblit.tickets.ITicketService; -import com.gitblit.tickets.ITicketService.TicketFilter; -import com.gitblit.tickets.QueryResult; -import com.gitblit.tickets.TicketIndexer.Lucene; import com.gitblit.tickets.BranchTicketService; -import com.gitblit.tickets.TicketLabel; -import com.gitblit.tickets.TicketMilestone; -import com.gitblit.tickets.TicketNotifier; import com.gitblit.utils.JGitUtils; import com.gitblit.utils.XssFilter; import com.gitblit.utils.XssFilter.AllowXssFilter; @@ -70,7 +58,7 @@ import com.gitblit.utils.XssFilter.AllowXssFilter; public class UITicketTest extends GitblitUnitTest { private ITicketService service; - final String repoName = "UITicketTest.git"; + static final String repoName = "UITicketTest.git"; final RepositoryModel repo = new RepositoryModel(repoName, null, null, null); protected ITicketService getService(boolean deleteAll) throws Exception { @@ -99,7 +87,9 @@ public class UITicketTest extends GitblitUnitTest { protected IStoredSettings getSettings(boolean deleteAll) throws Exception { File dir = new File(GitBlitSuite.REPOSITORIES, repoName); if (deleteAll) { - FileUtils.deleteDirectory(dir); + if (dir.exists()) { + FileUtils.delete(dir, FileUtils.RECURSIVE | FileUtils.RETRY); + } JGitUtils.createRepository(GitBlitSuite.REPOSITORIES, repoName).close(); } @@ -114,6 +104,15 @@ public class UITicketTest extends GitblitUnitTest { return settings; } + @AfterClass + public static void deleteUITicketTestRepo() throws IOException { + //delete the UITicketTest.git folder, at end of the test + File dir = new File(GitBlitSuite.REPOSITORIES, repoName); + if (dir.exists()) { + FileUtils.delete(dir, FileUtils.RECURSIVE | FileUtils.RETRY); + } + } + @Before public void setup() throws Exception { service = getService(true); @@ -148,4 +147,4 @@ public class UITicketTest extends GitblitUnitTest { return change; } -} \ No newline at end of file +} -- 2.39.5