How to ensure IJobChangeListener completion?

[basilevs]

RCPTT is a platform for testing Eclipse-based UI applications. One of its features is an option to wait for completion of any background tasks before proceeding to the next test step. This imitates user behavior, where after an interaction user waits for all relevant changes to happen on screen and then either proceeds to the next test step or verifies the application state.

A part of this feature waits for background jobs to complete. A few years back, this feature has started to produce inconsistent results. It turns out, we relied on IJobChangeListeners to detect job completion. This is incorrect, as job listeners are themselves a background operation, that should block a next test step from starting. Relying on one listener to wait for completion of all listeners is futile.

I investigated the matter recently and has tried a few things to restore the feature.

Job.join() itself relies on listeners and exits before listeners complete.
A short delay after last job execution does not help, as some listeners are long running.
There is no public API to observe listeners progress.

I was forced to use private methods via reflection to wait for listeners completion.

Is there any API to join a job including its listeners? How are Job framework clients supposed to know that side effects from running the job would no longer happen?

job.addListener(new JobChangeAdapter() {
    void done(IJobChangeEvent event) {
           button.getDisplay().syncExec(() -> {
               button.setEnabled(true);
           });
    }
});
job.schedule()
job.join();
button.getDisplay().syncExec(() -> {
    click(button); // fails, as the button is still disabled
});

[Bananeweizen]

I'm not really involved in that topic, so pardon, if this is stupid: Would it help to replace the JobManager instance reflectively with another instance that can track job progress, just during the test execution? My rationale is basically that you might "design" the JobManager as you need it, if you have to use reflection anyway.

[basilevs]

Yes it would, but this solution would be no less invasive than my current approach with same compatibility implications and risks.

[laeubi]

Sadly there is no such API. I myself struggled a lot with this already and actually "all jobs finished" is nothing you can really ever be wait for because jobs might be scheduled any time.

So from what you described, I would register one global Job listener, and then record any Jobs that are started between two steps and join / wait on them because that are the jobs that have been triggered by the current step.

Depending on your usecase you probably can use:

IJobManger#suspend(); <--- this will prevents new jobs from being scheduled
IJobManger#join(null, null); <--- this will wait for any jobs to finish
... do something ...
IJobManger#resume(); <-- this will resume job executions
... repeat the steps above ...

[basilevs]

Imagine application code registers a listener A, RCPTT registers a listener B to monitor Job execution. There is no guarantee, that once listener B fires, listener A had already been processed. There is no way to ensure completion of listener by adding another listener. And yes, RCPTT adds a global listener. No notification scheme would allow it to ensure its own listeners fire after all the others. Liistener execution order is undefined. Listener registration order can't be enforced.

[laeubi]

As said there is no such "no jobs will execute" as Jobs can be scheduled any time and rescheduled as well. So I don't understand why you want to "wait" for the listeners that makes no sense, as mentioned above you can suspend the manager to prevent it from scheduling new jobs so you can make sure that current running jobs complete and you can take some action (for example examine pending jobs) before resume the execution.

So from a test framework point of view, the only useful thing one might assert is that after a given step one suspends the manager waits for completion of current steps, then wait for a given amount that no new jobs arrive and either fail or restart the cycle, but as said it might be that a job is reschedule every second so you never reach the state where "nothing" happens.

[basilevs]

RCPTT effectively suspends UI thread (works in Display.sleep() method), so unless raw Threads are running, there is nothing that can schedule a Job unexpectedly.
Again, the goal is not to stop a Job form execution, it is to run all the jobs to completion ensuring, that all effects test may need are applied. This was done by RCPTT years ago with some caveats pertaining to long-running jobs and this listener bug.

[laeubi]

As said this is not possible as Jobs can be rescheduled (without creating a new one) or might be created on timed events (not triggered by UI) and so on. While for an UI Test application it makes sense to try control the UI thread or sync in other means with the UI, the Job Framework is made specifically for asynchronous execution, also it is not the only way to perform background work.

A test should never depend on internal implementation details (what is that Jobs are used, or even parallel streams or threads) instead one needs to wait on the user visible result, e.g the state of a control changes (or not) within a given time-frame.

As mentioned earlier, what sometimes could be useful is to wait for a given group of jobs (e.g. the autobuild jobs), but also here the better and more flexible approach would be to wait for the assertion become true.

[basilevs]

RCPTT does indeed have a limitation that raw Java Threads are not taken into account. As a well behaved Eclipse Platform UI application should not use those, this is not a concern. There are no timed events that are conventionally done in Eclipse outside of Job Framework and Display, which are both handled by RCPTT.
Test should not depend on internal implementation of the application under test. But Eclipse platform is not the application under test, it is a framework the application is running in. Just as mocks are suitable replacement for external dependencies of a component under test, RCPTT is free to mess with the Eclipse Platform to speed up and stabilize tests of actual application.

Unfortunately, there is no control to wait for - all jobs are shown in Progress view, but some of them are irrelevant (old jobs started before the test, or jobs running for too long, which have to be ignored for the test to continue). Also, is some contexts, Progress View is not available and can't be relied upon.
While a human tester may be capable of recognizing a test-specific termination condition for the background task, it is not an easy thing to do - it is not always clear which control changes it state last and explicitly selecting controls and writing waiting loops is cumbersome. The main feature of RCPTT is an ability to record a normal user interaction as a test, and asking user to determine a control to wait for detract from this experience.

[iloveeclipse]

job listeners are themselves a background operation

• Do you mean code that uses "new Thread" to perform work? There will be no way to detect that except you add debug listener to the JVM and watch new Threads creation events.
• If you mean "usual" way to do background operations in Eclipse, you could suspend job manager once you ready with test step and query job manager for running jobs of "null" family till it says "none".
• If you want to wait for SWT tasks, you can submit a "marker" asyncExec task and wait for its completion.

[laeubi]

Maybe also it is meant to wait for the Auto Build to complete ...

[basilevs]

job listeners are themselves a background operation

* Do you mean code that uses "new Thread" to perform work? There will be no way to detect that except you add debug listener to the JVM and watch new Threads creation events.

No (yes, in a sense, that Job Worker is started with a "new Thread"). IJobChangeListeners are executed (sometimes) in a Worker thread of JobManager after Job goes into NONE state. As those listeners may cause arbitrary effects, a test should wait for those effects to apply before resuming. If client code could somehow join the main loop of Worker thread, that would suite my needs.

* If you mean "usual" way to do background operations in Eclipse, you could suspend job manager once you ready with test step and query job manager for running jobs of "null" family till it says "none".

The goal is not to prevent jobs from starting, but to ensure they all run to completion, imitating normal use of application under test. Suspension of JobManager would prevent other jobs from starting, causing a deviation from normal operation.

* If you want to wait for SWT tasks, you can submit a "marker" asyncExec task and wait for its completion.

Even if we tried to use a marker task, it would not work, as additional tasks may be scheduled by already running tasks. Background activity would then continue after completion of marker task.
RCPTT covers UI activity by injecting it's code in Display.sleep() method, which is only run when all deferred UI tasks are complete.

[basilevs]

Auto build is just one of the jobs to wait for, the goal is to wait for all the jobs. And this is already done. "Done" listeners run outside of job context (when Job state is NONE), so unfortunately waiting for the job completion is not enough.

[iloveeclipse]

Is there any API to join a job including its listeners?

Any additional API that would add a listener for listening on listener updates may be used by other clients and so will be not sufficient for the purpose of listening on listeners.

The potential (theoretical) solution I see is to provide the possibility to set a different job manager implementation for tests, or possibility to supply a "delegate" job manager for tests (to not implement everything and to ensure the actual implementation one wants to test is not changed by the test).

[fedejeanne]

Sorry for jumping in so late but is waiting for the listeners really the solution?
I mean what happens if one changes the syncExec here to an asyncExec?

[laeubi]

@fedejeanne no I don't think this is a solution, I simply think the given example is invalid and that why I asked for a real world example not a synthetic one that shows bad behavior.

Why I think the example is invalid:

1. it performs a task of the job in a listener what should have been performed by the Job itself (see specifically there is a special org.eclipse.ui.progress.UIJob if you want to run in the UI)
2. it performs blocking operations in a listener (listeners should return fast and never block for unknown amount of time)
3. beside that, and I have mentioned it already multiple times, waitForAllRelatedActivityToStop is something that can't be deduced, because a Job is not a related activity here, it is a sideeffect

[laeubi]

This is how one would actually use the job framework:

Button button = ...;
Job job = new Job("Test") {

	@Override
	protected IStatus run(IProgressMonitor monitor) {
		// do what ever you want
		// ....
		button.getDisplay().syncExec(() -> {
			button.setEnabled(true);
		});
		return Status.OK_STATUS;
	}

};
job.addJobChangeListener(new JobChangeAdapter() {
	@Override
	public void done(IJobChangeEvent event) {
		button.getDisplay().asyncExec(() -> {
			click(button);
		});
	}
});
job.schedule();

[fedejeanne]

@laeubi thank you for the snippet!

I agree with @laeubi here: it seems to me that adding a method to wait for listeners is a bad idea and only meant to compensate for bad practices in the user code.

I can of course understand that RCPTT needs that because it should work even with (sorry for being blunt) poorly programmed client code, but I don't think providing such a method as part of the API is a good idea because it encourages bad practices.

My naive approach would be to:

1. Emphasize (in the documentation) that listeners are not to be misused as "continuation work" and that joining the job does not wait for its listeners.
2. Promote better practices for client code (like the snippet above), even make it easier by providing ways to cleanly "attach" more jobs to a job (via the "family" / "job group" maybe?)

[iloveeclipse]

• Emphasize (in the documentation) that listeners are not to be misused as "continuation work" and that joining the job does not wait for its listeners.

Sorry, using official API is not a misuse by definition. Typically "continuation work" is not don in the job because the listener and the job are from different modules / projects so one can't simply "enable the button" in the same job that has nothing to do with the button because it simply doesn't know anything about button at all.

• Promote better practices for client code (like the snippet above)

Snippet above is not a best practice because calling syncExec() from a job should be avoided in all cases. The reason is that syncExec() is acquiring a lock and the job itself might have already have one or multiple locks taken => straight forward road to deadlocks.

make it easier by providing ways to cleanly "attach" more jobs to a job (via the "family" / "job group" maybe?)

This is what the listeners API is for. There is no "family" API yet as a "family" is a very generic concept (can be null, String or whatever). JobGroup has already join().

[basilevs]

Yes, it is not my snippet :-)

Oops, well, anyway, it does not use syncExec in a listener

[basilevs]

On attaching jobs: I've missed the point, nevermind.

[fedejeanne]

@iloveeclipse what I meant by "misuse" is that "one should not try to 'expand' (i.e. attach more work to) a job by using listeners because listeners don't have the same lifecycle a job has". A job is done as soon as its run method finishes executing (listeners don't have a say in that) and therefore joining the job does not wait for the listeners (and it shouldn't!).

If one had an easier way to add more work to a job or at least to know what to wait for when one triggers that job then RCPTT could use that.

For example, if one could do stuff like:

job.attach( // attach another job. The current job only finishes when all attached jobs finish.
	Job.create("Activate button", m -> { button.setEnabled(true); }));

or something similar then calling job.join() would guarantee that all attached jobs finished their execution and make RCPTT happy (would it, @basilevs ?)

Please notice that one can still misuse this (fictional) solution by putting asynchronous work inside the attached job, so not even this solution is perfect. It would merely make it easier for client code to cleanly attach more work into the lifecycle of a job.

[basilevs]

@fedejeanne I don't think the suggested work attachment would work - if the new work is executed outside of SchedulingRule, user will question the need of a function so similar to the listeners. If it is executed inside scheduling rule, it can't be a job (because each job is supposed to have its own scheduling).
RCPTT won't be able to benefit because new APIs won't prevent usage of old API's where the problem originates.

[laeubi]

As said I don't see any reasons for a new API nor that RCPTT would require that, a user does not "wait" for background work to complete, and if buttons are disabled that fast that I can't capture this in a test-case because jobs are rescheduled that fast then the UI would simply be unusable.

Also note that I also used the "syncExec" because it was part of the example, in reality I would always ever only use Display.asyncExec anyways but I don't see how it makes things better here to use that in a listener.

Given that the example illustrate something one really see I would just expect the following:

1. The test code executes
2. RCPTT waits for Jobs to complete
3. the currently failing "click" would simply be a "wait a reasonable time for the button to be enabled and then click", I think at least SWTBot offers something like that.