Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test fails with I20230615-1800 #1150

Closed
iloveeclipse opened this issue Jun 16, 2023 · 19 comments
Closed

Test fails with I20230615-1800 #1150

iloveeclipse opened this issue Jun 16, 2023 · 19 comments
Assignees
Labels
bug Something isn't working regression Something was broken by a previous change
Milestone

Comments

@iloveeclipse
Copy link
Member

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

Wrong bundles loaded: - org.eclipse.jdt.apt.core - org.eclipse.jdt.apt.pluggable.core expected:<0> but was:<64>

java.lang.AssertionError: Wrong bundles loaded:
- org.eclipse.jdt.apt.core
- org.eclipse.jdt.apt.pluggable.core
expected:<0> but was:<64>

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

I assume (but have not verified that yet) eclipse-jdt/eclipse.jdt.debug#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.

@iloveeclipse iloveeclipse added bug Something isn't working regression Something was broken by a previous change labels Jun 16, 2023
@mickaelistria
Copy link
Contributor

I'll have a look and report possible remediations.

@mickaelistria
Copy link
Contributor

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
Copy link
Contributor

jukzi commented Jun 16, 2023

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

iloveeclipse added a commit to iloveeclipse/eclipse.jdt.core that referenced this issue Jun 18, 2023
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.core that referenced this issue Jun 18, 2023
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.core that referenced this issue Jun 18, 2023
iloveeclipse added a commit to iloveeclipse/eclipse.jdt.core that referenced this issue Jun 18, 2023
@mickaelistria
Copy link
Contributor

FWIW, as this issue has been worked around with eclipse-jdt/eclipse.jdt.ui@f575dff , 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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

jukzi commented Jun 19, 2023

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
Copy link
Member Author

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
Copy link
Contributor

jukzi commented Jun 19, 2023

You have to rebase on top of master.

it is - still failing

@iloveeclipse
Copy link
Member Author

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
Copy link
Contributor

jukzi commented Jun 19, 2023

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
Copy link
Member Author

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
Copy link
Member Author

@stephan-herrmann
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member Author

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

jarthana pushed a commit to jarthana/eclipse.jdt.core that referenced this issue Jun 27, 2023
jarthana pushed a commit to jarthana/eclipse.jdt.core that referenced this issue Jun 27, 2023
@stephan-herrmann
Copy link
Contributor

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 :)

mpalat pushed a commit to mpalat/eclipse.jdt.core that referenced this issue Jul 6, 2023
mpalat pushed a commit to mpalat/eclipse.jdt.core that referenced this issue Jul 6, 2023
subyssurendran666 pushed a commit to subyssurendran666/eclipse.jdt.core that referenced this issue Jul 18, 2023
subyssurendran666 pushed a commit to subyssurendran666/eclipse.jdt.core that referenced this issue Jul 18, 2023
robstryker pushed a commit to robstryker/eclipse.jdt.core that referenced this issue Jul 18, 2024
robstryker pushed a commit to robstryker/eclipse.jdt.core that referenced this issue Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working regression Something was broken by a previous change
Projects
None yet
Development

No branches or pull requests

4 participants