[Build] Run Maven build in parallel and only up until the verify phase

[HannesWell]

All the fragments can build in parallel so this should speed-up the build.

[github-actions]

Test Results

   299 files  ±0     299 suites  ±0   6m 10s ⏱️ +18s
 4 098 tests ±0   4 090 ✅ ±0   8 💤 ±0  0 ❌ ±0 
12 206 runs  ±0  12 133 ✅ ±0  73 💤 ±0  0 ❌ ±0 

Results for commit 48447f9. ± Comparison against base commit 31e1414.

♻️ This comment has been updated with latest results.

[HannesWell]

Looks like the test become flaky if run in parallel. Does anybody has experience with that?
Furthermore the runtime gain does not look as big as expected on the first runs.

[iloveeclipse]

I assume parallel tests do not run in different VNC clients? I don't know how you run UI tests in parallel, probably on different displays on Linux but no idea about Windows. Usually UI tests are very sensible, as all of UI inputs/outputs are "singletons" (clipboard, keyboard, mouse, application focus), so UI tests running in parallel on same machine most likely will be affected by that.

[HeikoKlare]

You would need to ensure that all bundles executing UI tests (namely org.eclipse.swt.tests and org.eclipse.swt.tests.win32) are not executed in parallel, for the reasons that @iloveeclipse mentioned. The tests in each bundle are still executed sequentially. The Linux and Macs builds are not be affected, because the OS-specific Mac tests are empty and the ones for GTK do not test any UI.

Maybe you know that @laeubi also did some according experiments for Platform UI (see eclipse-platform/eclipse.platform.ui#921).

[laeubi]

It all depends on the test, e.g. if you have tests that use a shared resource (like clipboard) one might want to write them in a way that it synchronize on that resource. e.g Junit5 has @ResourceLock.

For different bundles surefire has a special settings reactorConcurrencyLevel that limits the number of parallel executing reactor projects that use tests.

[HannesWell]

Thank you for all the input.

For different bundles surefire has a special settings reactorConcurrencyLevel that limits the number of parallel executing reactor projects that use tests.

Unfortunately org.eclipse.swt.tests uses Maven-surefire to run the tests. So even if we would configure the org.eclipse.swt.tests.win32 accordingly it would not prevent concurrent test execution.

One hack might be to have an explicit dependency of org.eclipse.swt.tests.win32 to org.eclipse.swt.tests.
This way they cannot be built together.

[HannesWell]

One hack might be to have an explicit dependency of org.eclipse.swt.tests.win32 to org.eclipse.swt.tests.
This way they cannot be built together.

Looks like this worked (at least in the last pass).
Its not nice, but I would not be concerned to much about such such dependency for a test. I just wonder if we should add it to the other platform-specific tests just in case they also use resources in the future?

[laeubi]

One hack might be to have an explicit dependency of org.eclipse.swt.tests.win32 to org.eclipse.swt.tests.
This way they cannot be built together.

You mean in parallel ;-)

[HannesWell]

One hack might be to have an explicit dependency of org.eclipse.swt.tests.win32 to org.eclipse.swt.tests.
This way they cannot be built together.

You mean in parallel ;-)

Yes. Meant at the same time, in parallel. Not together^^

[HannesWell]

I just wonder if we should add it to the other platform-specific tests just in case they also use resources in the future?

Added it now for all.

[HeikoKlare]

The problem with such a dependency is that it only makes sense in the context of a parallel Maven build. It does something else than what these dependency specifications are supposed to be used for, so no one except the ones involved in this PR will know that (and probably even they will forget), so there will be people trying to understand the necessity for this dependency and eventually even remove it.

I would at least suggest to add a comment in the Manifest that explains the necessity for the dependency.

[laeubi]

I would at least suggest to add a comment in the Manifest that explains the necessity for the dependency.

Manifest can't have comments. Another alternative would be additional.bundles in the build.properties or a dependency in the pom.xml or a p2.inf

[HeikoKlare]

Manifest can't have comments

Just out of interest: did that change at some point in time? Because in a former project, several years ago, I had comments in a Manifest (but I do not remember the syntax).

[HannesWell]

Manifest can't have comments

Just out of interest: did that change at some point in time? Because in a former project, several years ago, I had comments in a Manifest (but I do not remember the syntax).

It didn't change. But an OSGi framework is supposed to ignore Headers it doesn't understand. So you can have a Comment: Some remark header as long as all characters are permitted.

In general I agree with your concern, that's why I added an explanation in the commit message (that is then hopefully found).
But moving it to the build.properties is probably the best solution here.
Will prepare a follow-up tonight.

[laeubi]

A header is not a comment... manifests are meant to be processed by tools not humans that why the specification is rather "odd" and its wide use makes its near to impossible to change (e.g. the stupid no more than 72 bytes (not characters) rule) even though I don't know any manifest parser that enforces the rule (including java)

[HeikoKlare]

Thanks for clarification. Now I remember that we used a Comment: ... header, knowing that it was actually a misuse.

[HannesWell]

A header is not a comment... manifests are meant to be processed by tools not humans

That's right. But just because one can doesn't mean one should do something. :)

[HannesWell]

Created #1017.