search

eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
162 stars 130 forks source link

Test fails with I20230615-1800 #1150

Closed iloveeclipse closed 1 year ago

iloveeclipse commented 1 year ago

We have new reproducible test fails on all platform in JDT with I20230615-1800 build.

jdt.apt.tests

jdt.core.tests.model

One or many discrepancies, including: . ----------- Expected ------------ ------------ but was ------------ *.launch --------- Difference is ---------- expected:<[]> but was:<[*.launch]>

junit.framework.ComparisonFailure: One or many discrepancies, including: .
----------- Expected ------------

------------ but was ------------
*.launch
--------- Difference is ----------
expected:<[]> but was:<[*.launch]>
at org.eclipse.jdt.core.tests.junit.extension.TestCase.assertStringEquals(TestCase.java:266)
at org.eclipse.jdt.core.tests.junit.extension.TestCase.assertEquals(TestCase.java:242)
at org.eclipse.jdt.core.tests.dom.APIDocumentationTests.testJavaCoreAPI(APIDocumentationTests.java:302)

jdt.text.tests

java.lang.AssertionError: Wrong bundles loaded:

There were not many changes in last build, most notable:

I assume (but have not verified that yet) https://github.com/eclipse-jdt/eclipse.jdt.debug/pull/231 could be the one who caused the fails above, since it may affect which JVM is selected a s default, or some platform change.

mickaelistria commented 1 year ago

I'll have a look and report possible remediations.

mickaelistria commented 1 year ago

JavaCore.getDefaultOptions() seems to return 1.4 compliance when called early, and 17 when called later. I believe the DetectJVM job is working in parallel of those tests, so the default options are changed in the meantime. What I wonder is whether returning 1.4 compliance is a good thing in 1st place? What may cause that? Is this expected?

jukzi commented 1 year ago

during testAddExternalLibFolder6 the p.build(IncrementalProjectBuilder.FULL_BUILD, null); sets options to compliance 

Thread [main] (Suspended (breakpoint at line 5277 in JavaModelManager)) 
    JavaModelManager.setOptions(Hashtable<String,String>) line: 5277    
    JavaCore.setOptions(Hashtable<String,String>) line: 6260    
    JavaRuntime.updateCompliance(IVMInstall) line: 3390 
...
    JavaModelManager$CompilationParticipants.getCompilationParticipants(IJavaProject) line: 448 
...
    Project.internalBuild(IBuildConfiguration, int, String, Map<String,String>, IProgressMonitor) line: 609 
    Project.build(int, IProgressMonitor) line: 121  
    JavaProjectTests.testAddExternalLibFolder6() line: 240

but later in org.eclipse.jdt.core.tests.model.AbstractJavaModelTests.tearDown() defaultOptions are assumed

mickaelistria commented 1 year ago

FWIW, as this issue has been worked around with https://github.com/eclipse-jdt/eclipse.jdt.ui/commit/f575dff65e956c03d47fe54e6f52a461d8aaf598 , I don't plan to work on it further so it may be closed. But what @jukzi mentions in his last comment would probably still be worth investigating as it could be a bug that is not caused, but instead highlighted by the VM detection.

iloveeclipse commented 1 year ago

I don't plan to work on it further so it may be closed.

It can't, there are still test failures. I will check them later today.

But what @jukzi mentions in his last comment

I tried to understand this comment but failed. If there is something one can fix, please push a PR.

mickaelistria commented 1 year ago

It can't, there are still test failures

Ah OK. I overlooked and only saw the successful tests on repo-specific CI, let's wait for I-Build outcome then.

iloveeclipse commented 1 year ago

let's wait for I-Build outcome then.

Already there: https://download.eclipse.org/eclipse/downloads/drops4/I20230618-1800/testResults.php

APIDocumentationTests still fails, I wonder it is not visible in Jenkins, but might be the execution order matters. I assume org.eclipse.jdt.core.tests.dom need similar workaround as other tests.

jukzi commented 1 year ago

JavaProjectTests.testAddExternalLibFolder6 still fails. see for example https://github.com/eclipse-jdt/eclipse.jdt.core/pull/1119/checks?check_run_id=14325578340 you don't need to wait for I-Build: just rebase any of my commits

iloveeclipse commented 1 year ago

JavaProjectTests.testAddExternalLibFolder6 still fails. see for example https://github.com/eclipse-jdt/eclipse.jdt.core/pull/1119/checks?check_run_id=14325578340 you don't need to wait for I-Build: just rebase any of my commits

You have to rebase on top of master.

jukzi commented 1 year ago

You have to rebase on top of master.

it is - still failing

iloveeclipse commented 1 year ago

You have to rebase on top of master.

it is - still failing

Then it is probably caused by your PR, because other PR's don't see this fail.

jukzi commented 1 year ago

you can see in the build history that 1st build succeed. i.e. it is random. Also you have seen the error in other PRs -> it's unrelated.

iloveeclipse commented 1 year ago

Looks like APIDocumentationTests.testJavaCoreAPI started to fail randomly. In the system output we see https://download.eclipse.org/eclipse/downloads/drops4/I20230618-1800/testresults/ep429I-unit-cen64-gtk3-java17_linux.gtk.x86_64_17/org.eclipse.jdt.core.tests.dom.RunAllTests.txt

!MESSAGE SETUP testJavaCoreAPI
default value mismatch for org.eclipse.jdt.core.builder.resourceCopyExclusionFilter, real: *.launch, javadoc: 
default value mismatch for org.eclipse.jdt.core.builder.resourceCopyExclusionFilter, javadoc , real *.launch

The test reads default JavaOptions and compares it with expected values in the JavaCore.java file.

The default JDT core options do not contain *.launch value, but the options set after new JVM detection code calls JavaRuntime.initializeVMs() do contain this value, stack:

Thread [main] (Suspended (breakpoint at line 6260 in JavaCore)) 
    JavaCore.setOptions(Hashtable<String,String>) line: 6260    
    JavaRuntime.updateCompliance(IVMInstall) line: 3390 
    JavaRuntime.initializeVMs() line: 3303  
    JavaRuntime.getVMInstallTypes() line: 572   
    JavaRuntime.getVMInstallType(String) line: 453  
    DetectVMInstallationsJob.<init>() line: 48  
    LaunchingPlugin.start(BundleContext) line: 592  
    BundleContextImpl$2.run() line: 818 
    BundleContextImpl$2.run() line: 1   
    AccessController.executePrivileged(PrivilegedExceptionAction<T>, AccessControlContext, Class<?>) line: 807  
    AccessController.doPrivileged(PrivilegedExceptionAction<T>) line: 569   
    BundleContextImpl.startActivator(BundleActivator) line: 810 
    BundleContextImpl.start() line: 767 
    EquinoxBundle.startWorker0() line: 1032 
    EquinoxBundle$EquinoxModule.startWorker() line: 371 
    EquinoxBundle$EquinoxModule(Module).doStart(Module$StartOptions...) line: 605   
    EquinoxBundle$EquinoxModule(Module).start(Module$StartOptions...) line: 468 
    SecureAction.start(Module, Module$StartOptions...) line: 513    
    EclipseLazyStarter.postFindLocalClass(String, Class<?>, ClasspathManager) line: 117 
    ClasspathManager.findLocalClass(String) line: 570   
    EquinoxClassLoader(ModuleClassLoader).findLocalClass(String) line: 335  
    BundleLoader.findLocalClass(String) line: 397   
    BundleLoader.findClass0(String, boolean, boolean) line: 500 
    BundleLoader.findClass(String) line: 416    
    EquinoxClassLoader(ModuleClassLoader).loadClass(String, boolean) line: 168  
    EquinoxClassLoader(ClassLoader).loadClass(String) line: 520 
    EquinoxBundle.loadClass(String) line: 622   
    EquinoxRegistryStrategy(RegistryStrategyOSGI).createExecutableExtension(RegistryContributor, String, String) line: 196  
    ExtensionRegistry.createExecutableExtension(RegistryContributor, String, String) line: 920  
    ConfigurationElement.createExecutableExtension(String) line: 246    
    ConfigurationElementHandle.createExecutableExtension(String) line: 63   
    JavaModelManager$CompilationParticipants$1.run() line: 455  
    SafeRunner.run(ISafeRunnable) line: 45  
    JavaModelManager$CompilationParticipants.getCompilationParticipants(IJavaProject) line: 448 
    JavaBuilder.initializeBuilder(int, boolean) line: 629

Means two things: 1) I've failed to properly disable maven/tycho value for the JVM autodetection job 2) The APIDocumentationTests should first reset options to default 3) We've stumbled upon an ancient hack where JDT core defaults are updated in different plugin: org.eclipse.jdt.internal.launching.LaunchingPreferenceInitializer.initializeDefaultPreferences(). See https://bugs.eclipse.org/bugs/show_bug.cgi?id=260445 and Co for background 4) The hack is "activated" now because of the call to JavaRuntime.initializeVMs() at early startup

I'm looking for a suitable fix.

iloveeclipse commented 1 year ago
stephan-herrmann commented 1 year ago

For you entertainment there's loads of investigations regarding undesired initialization sequences of JDT/Core options in https://bugs.eclipse.org/bugs/show_bug.cgi?id=482991. A little quote from that bug:

Mere presence of o.e.jdt.launching can cause the call chain below, which will then cause the same dreaded test failure.

Sounds familiar?

iloveeclipse commented 1 year ago

Yes, same problem again. I haven't dig deeper to understand why all this dancing with this one extra jdt launching property was done at all, and why it couldn't be just a core default in first place. Probably some ancient compatibility issue or the like.

stephan-herrmann commented 1 year ago

JavaProjectTests.testAddExternalLibFolder6 still fails. see for example https://github.com/eclipse-jdt/eclipse.jdt.core/pull/1119/checks?check_run_id=14325578340 you don't need to wait for I-Build: just rebase any of my commits

I'm seeing that same failure locally, with jdt.{core,ui,debug} at current head of master. Looking at the fix here, it tells me, that only builds are fixed but not local test launches in Eclipse.

Needing to add the system property to every test launch that is potentially affected doesn't look like a good option. So, I'm re-opening this issue.

To my own surprise adding this to the static initializer of the affected test class fixes the failure for me:

System.setProperty("DetectVMInstallationsJob.disabled", "true");

But I'm not convinced, that a system property is a good way to control this in the first place. At least we'd need to find the best suitable location for this kludge.

Perhaps jdt.core needs an extension point for plugins wishing to initialize stuff after jdt.core is fully initialized, rather than activators launching initialization jobs to run at unknown point in time? Or an explicit callback for updating (compliance) options at a point in time that's suitable for jdt.core (i.e., after JavaCorePreferenceInitializer has run)?

iloveeclipse commented 1 year ago

Looking at the fix here, it tells me, that only builds are fixed but not local test launches in Eclipse.

Exact. That was the goal of this ticket, and it was done / closed.

Created new one https://github.com/eclipse-jdt/eclipse.jdt.core/issues/1190

stephan-herrmann commented 1 year ago

Looking at the fix here, it tells me, that only builds are fixed but not local test launches in Eclipse.

Exact. That was the goal of this ticket, and it was done / closed.

Created new one #1190

I'm fine with having this solved in a new issue, but now nobody involved in creating the regression will have it on their radar any more :)