Skip to content

Disable test_PlaceholderOnTopStackSwitch() on MacOS#3944

Open
trancexpress wants to merge 1 commit intoeclipse-platform:masterfrom
trancexpress:gh3893
Open

Disable test_PlaceholderOnTopStackSwitch() on MacOS#3944
trancexpress wants to merge 1 commit intoeclipse-platform:masterfrom
trancexpress:gh3893

Conversation

@trancexpress
Copy link
Copy Markdown
Contributor

PartOnTopManagerTest.test_PlaceholderOnTopStackSwitch() fails stabily in MacOS I-builds.

Since its unclear what is wrong, the test is now disabled to reduce noise in test results.

See: #3893

Copy link
Copy Markdown
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disabling the test means that the functionality is potentially broken on macOS or at least will further degrade/break due to lack of test coverage. In case that's fine for those consumers using macOS, I am fine with disabling the test. It might be reasonable to create an issue (or rephrease the existing one for the failing test) to document the disabled tests and the potentially broken functionality.

@Test
public void test_PlaceholderOnTopStackSwitch() {
// test fails stably on MacOS I-builds, its unclear how to fix it, see: https://github.com/eclipse-platform/eclipse.platform.ui/issues/3893
Assumptions.assumeFalse(Platform.OS.isMac());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am not mistaken, with JUnit 5 it should be possible to use @DisabledOnOs(OS.MACOS).

@trancexpress
Copy link
Copy Markdown
Contributor Author

trancexpress commented Apr 29, 2026

Disabling the test means that the functionality is potentially broken on macOS or at least will further degrade/break due to lack of test coverage.

I'm mostly worried because the test seems to work with tycho, as you've stated here: #3893 (comment)

As far as I-builds are concerned, we already don't know if the respective functionality works or not - the test fails in each build. But technically we would know if a PR breaks the test.

I don't know how to disable the test for I-builds only though... Maybe tycho sets some property we don't have in I-builds, I don't know.

Maybe we can add something in org.eclipse.e4.ui.tests/test.xml to distinguish the case?

        <target name="suite">
                <property name="sniff-folder" value="${eclipse-home}/databinding_sniff_folder" />
                <delete dir="${sniff-folder}" quiet="true" />
                <ant target="core-test" antfile="${library-file}" dir="${eclipse-home}">
                        <property name="data-dir" value="${sniff-folder}" />
                        <property name="plugin-name" value="${plugin-name}" />
                        <property name="classname" value="org.eclipse.e4.ui.tests.UIAllTests" />
                </ant>
        </target>

Here are the ENV variables set during the I-build: https://download.eclipse.org/eclipse/downloads/drops4/I20260428-1800/testresults/consolelogs/ep440I-unit-macosx-aarch64-java21_macosx.cocoa.aarch64_21_consolelog.txt

env.txt

No idea where to look for this during the CI job: https://github.com/eclipse-platform/eclipse.platform.ui/actions/runs/25103050435/job/73557140458?pr=3944

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026

Test Results

   852 files  ±0     852 suites  ±0   54m 52s ⏱️ -35s
 7 930 tests ±0   7 687 ✅ ±0  243 💤 ±0  0 ❌ ±0 
20 292 runs  ±0  19 637 ✅ ±0  655 💤 ±0  0 ❌ ±0 

Results for commit 407335f. ± Comparison against base commit f2e2f88.

♻️ This comment has been updated with latest results.

@trancexpress trancexpress force-pushed the gh3893 branch 3 times, most recently from 61cbeaf to d190a4c Compare April 29, 2026 14:12
@trancexpress
Copy link
Copy Markdown
Contributor Author

See test jobs from the PR, which should fail with the value of ANT_HOME in the message:

All of them fail without a message, i.e. ANT_HOME isn't set:

 Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 1.142 s <<< FAILURE! -- in org.eclipse.e4.ui.tests.workbench.PartOnTopManagerTest
org.eclipse.e4.ui.tests.UIAllTests.test_PlaceholderOnTopStackSwitch -- Time elapsed: 0.088 s <<< FAILURE!
org.opentest4j.AssertionFailedError
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:38)
	at org.junit.jupiter.api.Assertions.fail(Assertions.java:138)
	at org.eclipse.e4.ui.tests.workbench.PartOnTopManagerTest.test_PlaceholderOnTopStackSwitch(PartOnTopManagerTest.java:142)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

This is more complicated than just disabling the test. But if it passes in PR jobs, then better its disabled only for I-builds...

Best of course would be if the test gets fixed, but I have no MacOS machine and can't do much about that.

@HeikoKlare
Copy link
Copy Markdown
Contributor

@trancexpress
Copy link
Copy Markdown
Contributor Author

We already have (SWT) tests that check whether they are executed in Jenkins. Maybe you could adopt that: https://github.com/eclipse-platform/eclipse.platform.swt/blob/b40fe9428f8e3f280e7b6fa296c8bd809138ade7/tests/org.eclipse.swt.tests/JUnit%20Tests/org/eclipse/swt/tests/junit/SwtTestUtil.java#L135-L137

JOB_NAME is set in the I-builds, to this:

[echoproperties] env.JOB_NAME=AutomatedTests/ep440I-unit-macosx-aarch64-java21

I can check if its set to some other value during the PR jobs, but I'm not sure how reliable that will be? Though I'm also not sure how reliable checking ANT_HOME is.

@HeikoKlare
Copy link
Copy Markdown
Contributor

The SWT tests just check if JOB_NAME is set to anything, as Tycho builds do not have JOB_NAME set at all.

@trancexpress
Copy link
Copy Markdown
Contributor Author

trancexpress commented Apr 29, 2026

It does seem that one of the jobs has JOB_NAME set: https://github.com/eclipse-platform/eclipse.platform.ui/pull/3944/checks?check_run_id=73611378603

org.opentest4j.AssertionFailedError: eclipse.platform.ui/PR-3944
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:38)
	at org.junit.jupiter.api.Assertions.fail(Assertions.java:138)
	at org.eclipse.e4.ui.tests.workbench.PartOnTopManagerTest.test_PlaceholderOnTopStackSwitch(PartOnTopManagerTest.java:143)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

I'm not sure which job that is though, since the Windows/Linux/MacOS jobs show a pass status...

From my POV we can still ignore if JOB_NAME is set, since the MacOS job runs without JOB_NAME. WDYT @HeikoKlare ?

@HeikoKlare
Copy link
Copy Markdown
Contributor

From my POV we can still ignore if JOB_NAME is set, since the MacOS job runs without JOB_NAME.

Sounds reasonable. So guarding the test with JOB_NAME being empty or OS not being macOS, right?

PartOnTopManagerTest.test_PlaceholderOnTopStackSwitch() fails stabily in MacOS I-builds.

Since its unclear what is wrong, the test is now disabled to reduce noise in test results.

See: eclipse-platform#3893
* {@link #JENKINS_JOB_NAME_ENV_VAR} is set. Jenkins jobs are:
* <ul>
* <li>I-builds</li>
* <li>GitHub PR test job, excluding the Windows/Linux/MacOS test jobs ran with
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how to word this...

@trancexpress
Copy link
Copy Markdown
Contributor Author

@iloveeclipse please review this. The respective test has been failing in every I-build for about 3 weeks now, for MacOS ARM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants