Don't replace the monitor set by a group when starting a new job

[fedejeanne]

• Only set the progress monitor of the Job if it hasn't one already. This makes sure that the progress monitor set via Job::setProgressGroup is not replaced with a new one and ergo when the group is canceled, all the monitors in that group are canceled too.

• Add regression test: JobTest.testSetProgressGroup_progressGroupIsCanceled_monitorInJobIsCanceled()

• Simplify private method JobManager::createMonitor(Job) and remove the annotation @GuardedBy in it.

Fixes #664

[github-actions]

Test Results

       42 files  ±0         42 suites  ±0   57m 35s ⏱️ - 5m 23s
  3 771 tests +1    3 768 ✔️ +1    3 💤 ±0  0 ❌ ±0 
11 316 runs  +3  11 289 ✔️ +3  27 💤 ±0  0 ❌ ±0 

Results for commit 5b68124. ± Comparison against base commit 418fcfc.

♻️ This comment has been updated with latest results.

[HeikoKlare]

Changes look good to me. I obviously agree with the behavioral change in the JobManager, as stated in #654 (comment). I have some minor comments regarding code style within the added test.
And I would be in favor of separating the version bump from the functional changes in terms of different commits, but I am not sure if there is a policy for that.

[fedejeanne]

And I would be in favor of separating the version bump from the functional changes in terms of different commits, but I am not sure if there is a policy for that.

I'm not aware of one such policy but I'm 99% sure I have bumped the version in the same PR as the functional changes before. It makes sense to me since bumping the version is mandatory in this case :)

[mickaelistria]

There is no strict policy, but the recommendation is to make the change and the bump as 2 distinct commits in the same PR, so the functional part can be cherry-picked to older branches more easily if needed. But we usually won't reject a PR if the bump is included in the same commit as the code change.

[fedejeanne]

Thank you both for the hint, I'll take it into account from now on 👍

I created a PR to mention this in the documentation --> eclipse-platform/.github#151

[fedejeanne]

I had to bump another version so I split the commits anyway

[HeikoKlare]

Looks like the feature version of org.eclipse.core.runtime.feature has to be bumped as well, as the version of the included plugin org.eclipse.core.jobs was increased.
I wonder why this was not necessary when merging #625, as it bumped the version of org.eclipse.core.expressions, which is also included in that feature.

[fedejeanne]

Test failures documented in #678 and #668 (that last one should be fixed though --> rebasing)

[HeikoKlare]

The commit message of the version bump commit is not correct. It does neither bump the core.tests.runtime bundle version (I guess that was already done by another commit on which this PR was rebased) nor the version of the feature bundle.

I am not sure why the ToC tests (see #668) are failing again, as they succeeded in test runs this morning. There also seems to be some other configuration problem w.r.t. the changed jobs plug-in as the builds fail with validation errors:

Failed to execute goal org.eclipse.tycho:tycho-apitools-plugin:4.0.2:verify (verify) on project org.eclipse.core.jobs: Execute ApiApplication failed: Application returned exit code 10

The logs also show this, which I have not seen for the core.jobs plug-in in other builds yet:

The following implicit projects are not referenced: 
org.eclipse.platform:org.eclipse.core.jobs:eclipse-plugin:3.15.100-SNAPSHOT

[fedejeanne]

The commit message of the version bump commit is not correct. It does neither bump the core.tests.runtime bundle version (I guess that was already done by another commit on which this PR was rebased) nor the version of the feature bundle.

Correct, org.eclipse.core.tests.runtime was bumped in 8a3f6d1 and org.eclipse.core.runtime.feature in 5531532. I reworded the commit.

I am not sure why the ToC tests (see #668) are failing again, as they succeeded in test runs this morning. There also seems to be some other configuration problem w.r.t. the changed jobs plug-in as the builds fail with validation errors:

Failed to execute goal org.eclipse.tycho:tycho-apitools-plugin:4.0.2:verify (verify) on project org.eclipse.core.jobs: Execute ApiApplication failed: Application returned exit code 10

The logs also show this, which I have not seen for the core.jobs plug-in in other builds yet:

The following implicit projects are not referenced: 
org.eclipse.platform:org.eclipse.core.jobs:eclipse-plugin:3.15.100-SNAPSHOT

No idea why since my changes are rather simple, nothing I haven't done before. Let's see if it's a random failure, I just rebased.

[HeikoKlare]

Great, checks now run successfully.
The log message seems to only be related to a "cyclic dependency" as core.jobs has been bumped, the platform projects depend on equinox.registry which in turn depends on a package from core.jobs.