From 329d454fa4c86aa821a2a1decf3fc4ef793e3d6f Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Thu, 7 Jan 2016 16:55:42 +0100 Subject: [PATCH] [test stability] Make sure we don't miss index diff events The global listener to the indexDiffEntry should be added *before* reload job is triggered by the entry, otherwise it can happen that the job is done earlier. Also changed related test code to allow us better catch jobs etc. Change-Id: I0889c46b5577bf818658480270256ba634796608 Signed-off-by: Andrey Loskutov --- .../eclipse/egit/core/GitMoveDeleteHookTest.java | 16 ++++++------ .../indexdiff/IndexDiffCacheEntryTest.java | 5 ++-- .../internal/indexdiff/IndexDiffCacheTest.java | 30 ++++++++++++++-------- .../synchronize/GitSubscriberMergeContextTest.java | 7 ++--- .../org/eclipse/egit/core/test/TestRepository.java | 5 ++-- .../src/org/eclipse/egit/core/test/TestUtils.java | 24 ++++++++++++----- .../egit/core/test/op/AddOperationTest.java | 14 +++++----- .../core/internal/indexdiff/IndexDiffCache.java | 9 ++++--- .../internal/indexdiff/IndexDiffCacheEntry.java | 11 +++++++- 9 files changed, 77 insertions(+), 44 deletions(-) diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/GitMoveDeleteHookTest.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/GitMoveDeleteHookTest.java index a3e16f37c..354641c82 100644 --- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/GitMoveDeleteHookTest.java +++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/GitMoveDeleteHookTest.java @@ -94,8 +94,8 @@ public class GitMoveDeleteHookTest { SystemReader.setInstance(null); } - private TestProject initRepoInsideProjectInsideWorkspace() throws IOException, - CoreException { + private TestProject initRepoInsideProjectInsideWorkspace() + throws Exception { TestProject project = new TestProject(true, "Project-1", true, workspaceSupplement); File gitDir = new File(project.getProject().getLocationURI().getPath(), Constants.DOT_GIT); @@ -108,7 +108,7 @@ public class GitMoveDeleteHookTest { } private TestProject initRepoInsideProjectOutsideWorkspace() - throws IOException, CoreException { + throws Exception { TestProject project = new TestProject(true, "Project-1", false, workspaceSupplement); File gitDir = new File(project.getProject().getLocationURI().getPath(), @@ -121,12 +121,12 @@ public class GitMoveDeleteHookTest { } private TestProject initRepoAboveProjectInsideWs(String srcParent, String d) - throws IOException, CoreException { + throws Exception { return initRepoAboveProject(srcParent, d, true); } private TestProject initRepoAboveProject(String srcParent, String d, boolean insidews) - throws IOException, CoreException { + throws Exception { registerWorkspaceRelativeTestDir(srcParent); TestProject project = new TestProject(true, srcParent + "Project-1", insidews, workspaceSupplement); File gd = new File(insidews?workspace:workspaceSupplement, d); @@ -160,11 +160,11 @@ public class GitMoveDeleteHookTest { assertNotNull(dirCache.getEntry("file2.txt")); // Modify the content before the move testUtils.changeContentOfFile(project.getProject(), file, "other text"); - testUtils.waitForJobs(1000, 10000, JobFamilies.INDEX_DIFF_CACHE_UPDATE); + TestUtils.waitForJobs(500, 10000, JobFamilies.INDEX_DIFF_CACHE_UPDATE); file.delete(true, null); - testUtils.waitForJobs(1000, 10000, JobFamilies.INDEX_DIFF_CACHE_UPDATE); + TestUtils.waitForJobs(500, 10000, JobFamilies.INDEX_DIFF_CACHE_UPDATE); // Check index for the deleted file dirCache.read(); @@ -600,7 +600,7 @@ public class GitMoveDeleteHookTest { private void dotestMoveProjectWithinRepo(String srcParent, String srcProjectName, String dstParent, String dstProjecName, - String gitDir, boolean sourceInsideWs) throws IOException, CoreException { + String gitDir, boolean sourceInsideWs) throws Exception { String gdRelativeSrcParent = srcParent + srcProjectName + "/"; if (gdRelativeSrcParent.startsWith(gitDir)) gdRelativeSrcParent = gdRelativeSrcParent diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntryTest.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntryTest.java index be76713b8..91b77af4e 100644 --- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntryTest.java +++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntryTest.java @@ -24,6 +24,7 @@ import org.eclipse.egit.core.Activator; import org.eclipse.egit.core.JobFamilies; import org.eclipse.egit.core.test.GitTestCase; import org.eclipse.egit.core.test.TestRepository; +import org.eclipse.egit.core.test.TestUtils; import org.eclipse.jgit.lib.Repository; import org.junit.After; import org.junit.Before; @@ -178,7 +179,7 @@ public class IndexDiffCacheEntryTest extends GitTestCase { */ private void waitForJobs(long maxWaitTime, Object family) throws InterruptedException { - testUtils.waitForJobs(maxWaitTime, family); + TestUtils.waitForJobs(maxWaitTime, family); } private void cleanEntryFlags() { @@ -221,7 +222,7 @@ public class IndexDiffCacheEntryTest extends GitTestCase { boolean updateScheduled; public IndexDiffCacheEntry2(Repository repository) { - super(repository); + super(repository, null); } @Override diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheTest.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheTest.java index 275992352..4f7cc2160 100644 --- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheTest.java +++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheTest.java @@ -43,17 +43,32 @@ public class IndexDiffCacheTest extends GitTestCase { private AtomicReference indexDiffDataResult; + private IndexDiffChangedListener indexDiffListener; + @Override @Before public void setUp() throws Exception { super.setUp(); testRepository = new TestRepository(gitDir); repository = testRepository.getRepository(); + listenerCalled = new AtomicBoolean(false); + indexDiffDataResult = new AtomicReference<>(null); + indexDiffListener = new IndexDiffChangedListener() { + @Override + public void indexDiffChanged(Repository repo, + IndexDiffData indexDiffData) { + listenerCalled.set(true); + indexDiffDataResult.set(indexDiffData); + } + }; } @Override @After public void tearDown() throws Exception { + IndexDiffCache indexDiffCache = Activator.getDefault() + .getIndexDiffCache(); + indexDiffCache.removeIndexDiffChangedListener(indexDiffListener); testRepository.dispose(); repository = null; super.tearDown(); @@ -252,22 +267,15 @@ public class IndexDiffCacheTest extends GitTestCase { } private IndexDiffCacheEntry prepareCacheEntry() { + listenerCalled.set(false); + indexDiffDataResult.set(null); + IndexDiffCache indexDiffCache = Activator.getDefault() .getIndexDiffCache(); + indexDiffCache.addIndexDiffChangedListener(indexDiffListener); // This call should trigger an indexDiffChanged event IndexDiffCacheEntry cacheEntry = indexDiffCache .getIndexDiffCacheEntry(repository); - listenerCalled = new AtomicBoolean(false); - indexDiffDataResult = new AtomicReference( - null); - cacheEntry.addIndexDiffChangedListener(new IndexDiffChangedListener() { - @Override - public void indexDiffChanged(Repository repo, - IndexDiffData indexDiffData) { - listenerCalled.set(true); - indexDiffDataResult.set(indexDiffData); - } - }); return cacheEntry; } diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/synchronize/GitSubscriberMergeContextTest.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/synchronize/GitSubscriberMergeContextTest.java index d45d6f8cd..e92e41d70 100644 --- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/synchronize/GitSubscriberMergeContextTest.java +++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/synchronize/GitSubscriberMergeContextTest.java @@ -27,6 +27,7 @@ import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.NullProgressMonitor; import org.eclipse.egit.core.project.RepositoryMapping; import org.eclipse.egit.core.test.TestRepository; +import org.eclipse.egit.core.test.TestUtils; import org.eclipse.egit.core.test.models.ModelTestCase; import org.eclipse.egit.core.test.models.SampleResourceMapping; import org.eclipse.jgit.api.Git; @@ -370,7 +371,7 @@ public class GitSubscriberMergeContextTest extends ModelTestCase { setContentsAndCommit(testRepo, iFile1, branchChanges + initialContent1, "branch commit"); iFile2.delete(true, new NullProgressMonitor()); - testUtils.waitForJobs(500, 5000, null); + TestUtils.waitForJobs(500, 5000, null); assertFalse(iFile2.exists()); testRepo.addAndCommit(iProject, file2, "branch commit - deleted file2." @@ -379,7 +380,7 @@ public class GitSubscriberMergeContextTest extends ModelTestCase { testRepo.checkoutBranch(MASTER); iProject.refreshLocal(IResource.DEPTH_INFINITE, new NullProgressMonitor()); - testUtils.waitForJobs(500, 5000, null); + TestUtils.waitForJobs(500, 5000, null); assertTrue(iFile2.exists()); final String masterChanges = "some changes\n"; @@ -388,7 +389,7 @@ public class GitSubscriberMergeContextTest extends ModelTestCase { iProject.refreshLocal(IResource.DEPTH_INFINITE, new NullProgressMonitor()); // end setup - testUtils.waitForJobs(500, 5000, null); + TestUtils.waitForJobs(500, 5000, null); IMergeContext mergeContext = prepareModelContext(repo, iFile1, MASTER, BRANCH); IDiff node = mergeContext.getDiffTree().getDiff(iFile1); diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/TestRepository.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/TestRepository.java index d21a7a7e7..62ecd00fe 100644 --- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/TestRepository.java +++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/TestRepository.java @@ -504,12 +504,13 @@ public class TestRepository { * Connect a project to this repository * * @param project - * @throws CoreException + * @throws Exception */ - public void connect(IProject project) throws CoreException { + public void connect(IProject project) throws Exception { ConnectProviderOperation op = new ConnectProviderOperation(project, this.getRepository().getDirectory()); op.execute(null); + TestUtils.waitForJobs(50, 5000, null); } /** diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/TestUtils.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/TestUtils.java index bd3af8306..a6abb1c09 100644 --- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/TestUtils.java +++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/TestUtils.java @@ -313,9 +313,9 @@ public class TestUtils { * @param family * @throws InterruptedException */ - public void waitForJobs(long maxWaitTime, Object family) + public static void waitForJobs(long maxWaitTime, Object family) throws InterruptedException { - waitForJobs(50, maxWaitTime, family); + waitForJobs(100, maxWaitTime, family); } /** @@ -328,15 +328,15 @@ public class TestUtils { * can be null which means all job families * @throws InterruptedException */ - public void waitForJobs(long minWaitTime, long maxWaitTime, Object family) + public static void waitForJobs(long minWaitTime, long maxWaitTime, + Object family) throws InterruptedException { - Thread.sleep(minWaitTime); long start = System.currentTimeMillis(); + Thread.sleep(minWaitTime); IJobManager jobManager = Job.getJobManager(); - Job[] jobs = jobManager.find(family); - while (jobs.length > 0) { - Thread.sleep(100); + while (busy(jobs)) { + Thread.sleep(50); jobs = jobManager.find(family); if (System.currentTimeMillis() - start > maxWaitTime) { return; @@ -344,6 +344,16 @@ public class TestUtils { } } + private static boolean busy(Job[] jobs) { + for (Job job : jobs) { + int state = job.getState(); + if (state == Job.RUNNING || state == Job.WAITING) { + return true; + } + } + return false; + } + private static HashMap mkmap(String... args) { if ((args.length % 2) > 0) throw new IllegalArgumentException("needs to be pairs"); diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/AddOperationTest.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/AddOperationTest.java index b2fe65deb..5cd488aea 100644 --- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/AddOperationTest.java +++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/AddOperationTest.java @@ -133,9 +133,9 @@ public class AddOperationTest extends GitTestCase { testRepository.commit("first commit"); - assertEquals(file1.getLocalTimeStamp(), + assertEquals(file1.getLocalTimeStamp() / 1000, testRepository.lastModifiedInIndex(file1.getLocation() - .toPortableString())); + .toPortableString()) / 1000); Thread.sleep(1000); @@ -158,9 +158,11 @@ public class AddOperationTest extends GitTestCase { assertTrue(testRepository.inIndex(file2.getLocation() .toPortableString())); - assertEquals(file1.getLocalTimeStamp() / 10, - testRepository.lastModifiedInIndex(file1.getLocation().toPortableString()) / 10); - assertEquals(file2.getLocalTimeStamp() / 10, - testRepository.lastModifiedInIndex(file2.getLocation().toPortableString()) / 10); + assertEquals(file1.getLocalTimeStamp() / 1000, + testRepository.lastModifiedInIndex( + file1.getLocation().toPortableString()) / 1000); + assertEquals(file2.getLocalTimeStamp() / 1000, + testRepository.lastModifiedInIndex( + file2.getLocation().toPortableString()) / 1000); } } diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCache.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCache.java index b874f737b..3ac3ff217 100644 --- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCache.java +++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCache.java @@ -222,14 +222,15 @@ public class IndexDiffCache { IndexDiffCacheEntry entry; synchronized (entries) { entry = entries.get(repository); - if (entry != null) + if (entry != null) { return entry; - if (repository.isBare()) + } + if (repository.isBare()) { return null; - entry = new IndexDiffCacheEntry(repository); + } + entry = new IndexDiffCacheEntry(repository, globalListener); entries.put(repository, entry); } - entry.addIndexDiffChangedListener(globalListener); return entry; } diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntry.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntry.java index cd6f22bab..e1791d991 100644 --- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntry.java +++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/indexdiff/IndexDiffCacheEntry.java @@ -44,6 +44,7 @@ import org.eclipse.egit.core.internal.CoreText; import org.eclipse.egit.core.internal.job.RuleUtil; import org.eclipse.egit.core.internal.trace.GitTraceLocation; import org.eclipse.egit.core.internal.util.ProjectUtil; +import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.dircache.DirCacheIterator; import org.eclipse.jgit.errors.IndexReadException; @@ -96,9 +97,16 @@ public class IndexDiffCacheEntry { /** * @param repository + * @param listener + * can be null */ - public IndexDiffCacheEntry(Repository repository) { + public IndexDiffCacheEntry(Repository repository, + @Nullable IndexDiffChangedListener listener) { this.repository = repository; + if (listener != null) { + addIndexDiffChangedListener(listener); + } + indexChangedListenerHandle = repository.getListenerList().addIndexChangedListener( new IndexChangedListener() { @Override @@ -113,6 +121,7 @@ public class IndexDiffCacheEntry { scheduleReloadJob("RefsChanged"); //$NON-NLS-1$ } }); + scheduleReloadJob("IndexDiffCacheEntry construction"); //$NON-NLS-1$ createResourceChangeListener(); if (!repository.isBare()) { -- 2.11.4.GIT