Community
Participate
Working Groups
While removing "new index" code via bug 572978 I've naively replaced the code that enabled/disabled indexer and called waitForIndex() with the respective alternatives of the "old indexer". That was naive because the "old indexer" code *really* works, instead the "new indexer" code used overall in JDT tests was functionally no-op and never switched indexer off or on or waited for the index! I guess that would explain sporadic issues we saw in bug 459092 and also other oddities and random JDT test failures. Means, API below is non functional on master (but used in tests): Indexer.getInstance().waitForIndex(null) Indexer.getInstance().enableAutomaticIndexing() I will try to replace the calls with: JavaModelManager.getIndexManager().disable() JavaModelManager.getIndexManager().enable() and a new implementation for JavaModelManager.getIndexManager().waitForIndex(); As of today, following tests are failing on master if I do this: JavaSearchBugs8Tests.testBug400905_0006 JavaSearchBugs8Tests.testBug400905_0007 JavaSearchBugs8Tests.testBug400905_0008 JavaSearchBugs8Tests.testBug400905_0010 JavaSearchBugs8Tests.testBug400905_0013a JavaSearchBugs8Tests.testBug400905_0014 JavaSearchBugs8Tests.testBug400905_0016 JavaIndexTests.testNonExistentIndexRestart TypeHierarchyTests.testProgressWhileIndexing
OK, if I *properly* wait for index I see that JavaIndexTests.testNonExistentIndexRestart always fail, and that exists since long time, see bug 534548. The "random" fails from bug 534548 are resulting from the fact, that indexing of the jar in question happens asynchronously and either the time stamp is same because indexing is faster as file system resolution or the indexer runs a moment after the test assertion. With proper waiting on the indexer we see the test always failing.
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/182736
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/182736 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=09687d64ecd9d0744a335c284c64c99f491d7e64
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/182739
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/182739 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=ed3b29ca4b274ab669e5f48e9fc6de8f7f42a894
org.eclipse.jdt.core.tests.dom did not finished in SDK builds, and that was unnoticed => bug 574649. I will push the patch for org.eclipse.jdt.core.tests.dom as soon as tests finish on my machine without timeout :-)
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/182752
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/182752 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=576a7c5093667ddea63abe71c6962928fba8d394
(In reply to Andrey Loskutov from comment #6) > org.eclipse.jdt.core.tests.dom did not finished in SDK builds, and that was > unnoticed => bug 574649. > > I will push the patch for org.eclipse.jdt.core.tests.dom as soon as tests > finish on my machine without timeout :-) Works in I20210705-0600, see https://download.eclipse.org/eclipse/downloads/drops4/I20210705-0600/testresults/html/org.eclipse.jdt.core.tests.model_ep421I-unit-cen64-gtk3-java11_linux.gtk.x86_64_11.html
All tests working again in I20210705-0600, see https://download.eclipse.org/eclipse/downloads/drops4/I20210705-0600/testResults.php
Verified with Eclipse. Version: 2021-09 (4.21) Build id: I20210706-1800 [with the test results page of the above, in fact.]
@All JDT committers: here is a summary to document current behavior / possible side effects of this change. org.eclipse.jdt.core.tests.junit.extension.TestCase is now disabling the indexer before the test in setUp() and enables indexer after the test in tearDown() via following API's: JavaModelManager.getIndexManager().disable() JavaModelManager.getIndexManager().enable() To customize that behavior for single tests, protected field indexDisabledForTest and public method isIndexDisabledForTest() are introduced. The isIndexDisabledForTest() method is also used in some other utility functions provided by JDT test framework, where access to the index may happen, especially if calling JavaModelManager.getIndexManager().waitForIndex(isIndexDisabledForTest(), null). The new org.eclipse.jdt.internal.core.search.indexing.IndexManager.waitForIndex(boolean, IProgressMonitor) method allows clients to wait for the indexer tasks - but it also will block, if the indexer is disabled. *** Here is what now important *** If a test extends org.eclipse.jdt.core.tests.junit.extension.TestCase but relies on the indexer work to be done, without further work this test will hang forever - because waiting on indexer that is disabled is obviously a bad idea! So if some JDT test is stuck in wait() at org.eclipse.jdt.internal.core.search.processing.JobManager.performConcurrentJob(IJob, int, IProgressMonitor), it is a sign that the test depends on indexer to be executed and the indexer must be enabled for the test. To avoid that endless wait for an indexer that is not running, a test class can either decide to set the indexDisabledForTest field value to "false" in setUp() method for all tests in the class or inside the test for some particular code path that requires working indexer. *If* test code changes indexDisabledForTest during the test, at the end of the test, indexDisabledForTest must be set to the original value, otherwise IndexManager.enable() call on tearDown() will be not executed. I *hope* that with proper indexer enablement/waiting, the JDT tests would run now more stable. Tests that never relied on indexer and run now with *really* disabled indexer, shouldn't notice any difference. Tests that rely on indexer and call waitForIndex() might run now a bit longer, but that is the price for a stable test execution. Having tests run a bit slower but not randomly failing is better than have randomly failing tests that need to be restarted again and again to get the result.
still got a failing JavaIndexTests.testPlatformJarIndexFile() https://ci.eclipse.org/jdt/job/eclipse.jdt.core-Gerrit/5268/ ----------- Expected ------------ platform:/resource/ForIndex/Test.index.zip!/Test.index ------------ but was ------------ file:/home/jenkins/agent/workspace/eclipse.jdt.core-Gerrit/org.eclipse.jdt.core.tests.model/target/work/data/.metadata/.plugins/org.eclipse.jdt.core/2956376118.index is that related?
(In reply to Jörg Kubitz from comment #13) > still got a failing JavaIndexTests.testPlatformJarIndexFile() > https://ci.eclipse.org/jdt/job/eclipse.jdt.core-Gerrit/5268/ > > ----------- Expected ------------ > platform:/resource/ForIndex/Test.index.zip!/Test.index > ------------ but was ------------ > file:/home/jenkins/agent/workspace/eclipse.jdt.core-Gerrit/org.eclipse.jdt. > core.tests.model/target/work/data/.metadata/.plugins/org.eclipse.jdt.core/ > 2956376118.index > > is that related? Most likely not, but I can't say without debugging. JavaIndexTests extend AbstractJavaSearchTests and there is indexer enabled (== same behavior as before).
There is one warning that got introduced by this commit: Description Resource Path Location Type The type ITypeNameRequestor is deprecated FullSourceWorkspaceSearchTests.java /org.eclipse.jdt.core.tests.performance/src/org/eclipse/jdt/core/tests/performance line 28 Java Problem [ref https://bugs.eclipse.org/bugs/show_bug.cgi?id=574699#c2 here as well]
