Avoid running addons SWT tests twice in the Tycho build

[vogella]

The org.eclipse.e4.ui.workbench.addons.swt.test bundle defined an AllTests @suite while Tycho also scans test classes by name, so MaximizeBugTest, MaximizePartSashContainerPlaceholderTest and CleanupAddonTest each ran twice per build. For MaximizeBugTest that wasted about 35 seconds on every platform. This removes the redundant AllTests suite and renames MaximizableChildrenTag to MaximizableChildrenTagTest so the scanner runs it directly, leaving every test class to run exactly once. The bundle has no test.xml, so the Ant nightly build is unaffected.

[github-actions]

Test Results

   864 files  ±  0     864 suites  ±0   53m 28s ⏱️ + 8m 36s
 8 050 tests ±  0   7 807 ✅ ±  0  243 💤 ±0  0 ❌ ±0 
20 052 runs   - 532  19 398 ✅  - 531  654 💤  - 1  0 ❌ ±0 

Results for commit ceacb75. ± Comparison against base commit 4f9ee7a.

This pull request removes 4 and adds 4 tests. Note that renamed tests count towards both.

org.eclipse.e4.ui.workbench.addons.minmax.MaximizableChildrenTag ‑ testAreaMax
org.eclipse.e4.ui.workbench.addons.minmax.MaximizableChildrenTag ‑ testMainPartStackMax
org.eclipse.e4.ui.workbench.addons.minmax.MaximizableChildrenTag ‑ testPartStack1Max
org.eclipse.e4.ui.workbench.addons.minmax.MaximizableChildrenTag ‑ testPartStack2Max

org.eclipse.e4.ui.workbench.addons.minmax.MaximizableChildrenTagTest ‑ testAreaMax
org.eclipse.e4.ui.workbench.addons.minmax.MaximizableChildrenTagTest ‑ testMainPartStackMax
org.eclipse.e4.ui.workbench.addons.minmax.MaximizableChildrenTagTest ‑ testPartStack1Max
org.eclipse.e4.ui.workbench.addons.minmax.MaximizableChildrenTagTest ‑ testPartStack2Max

[HeikoKlare]

Is there a reason for not having this test bundle included in the I-Build test execution? For that, a test suite like the AllTests would be required,

[vogella]

Good question for releng, AFAICS we have 5-7 test bundles without that test.xml. But this should be discussed separately not in this PR, the test.xml misses since decades.

[akurtakov]

The "standard" way to prevent tests being run twice(or even more) due to suites and autodiscovery is to configure tycho to run the suite as it's done in https://github.com/akurtakov/eclipse.platform.ui/blob/ab2fd412c92a9b555d772e231bbb9bb79890aba3/tests/org.eclipse.ui.tests/build.properties#L32

[vogella]

The "standard" way to prevent tests being run twice(or even more) due to suites and autodiscovery is to configure tycho to run the suite as it's done in https://github.com/akurtakov/eclipse.platform.ui/blob/ab2fd412c92a9b555d772e231bbb9bb79890aba3/tests/org.eclipse.ui.tests/build.properties#L32

Ah, sorry didn't see that comment early enough. In case you what this approach, let me know, I can change the handling (alternatively you can also just implement it).

[merks]

If you blink during an evening, you just never know what will be merged a few minutes later. There’s something wrong with that, isn’t there?

[akurtakov]

Please change

The "standard" way to prevent tests being run twice(or even more) due to suites and autodiscovery is to configure tycho to run the suite as it's done in https://github.com/akurtakov/eclipse.platform.ui/blob/ab2fd412c92a9b555d772e231bbb9bb79890aba3/tests/org.eclipse.ui.tests/build.properties#L32

Ah, sorry didn't see that comment early enough. In case you what this approach, let me know, I can change the handling (alternatively you can also just implement it).

Please change it.

[vogella]

If you blink during an evening, you just never know what will be merged a few minutes later. There’s something wrong with that, isn’t there?

The PR validation has been failing in the repo for a week and I haven't seen anyone else pick it up. Fixing the failing tests and removing the duplicate test execution (which went unnoticed by releng for years) is safe and reduces the Mac timeout kills, so I think it's worth merging.

Alex's alternative is fine to implement, but as far as I can tell it leads to the same outcome.

[vogella]

Please change it.

Will do, PR to arrive soon

[vogella]

FYI - The original suite omitted StackDropAgentTest so "only" using the test suite would be wrong, I will include it in my upcoming PR

[vogella]

#4106, thanks @akurtakov for the tip with pom.model.property.testClass I did not know about this one.

[HeikoKlare]

A comment with a concern was even raised before the PR was merged pretty fast (again): #4103 (comment)
Sad to see that it was effectively just ignored and the PR still merged right away, though there would have been proper alternatives...

[vogella]

A comment with a concern was even raised before the PR was merged pretty fast (again): #4103 (comment) Sad to see that it was effectively just ignored and the PR still merged right away, though there would have been proper alternatives...

Not ignored, just not seen and the PR for changing it is already there.

[HeikoKlare]

Not ignored, just not seen and the PR for changing it is already there.

That's strange because you (or an agent of yours) answered my comment before you (or an agent of yours) merged this PR.

[iloveeclipse]

you (or an agent of yours)

Gender-neutral language is so yesterday 🥇

[vogella]

Not ignored, just not seen and the PR for changing it is already there.

That's strange because you (or an agent of yours) answered my comment before you (or an agent of yours) merged this PR.

Your comment was answered check the time line (test.xml was missing should be discussed...). But I agree I should having given you time to response to that.