Bugzilla – Bug 5412
DeciderTests#testDeciderExitStatusIsSetOnJobContext and step looping
Last modified: 2014-01-03 03:47:20 UTC
In DeciderTests#testDeciderExitStatusIsSetOnJobContext, the job that it executes runs (decider_transitions_on_restart.xml) is configured with steps that form a loop (step1 --> decider1 --> flow1 --> decider1, etc). Per sections 8.2, 8.2.5, 8.3, 8.4, and 8.6.1 of the spec, this is not allowed.
I can't argue with your reading of the spec.
I'm responsible for this, I wrongly thought that it was only re-execution of a step that was disallowed.
You might find my rationale for thinking this interesting, though.
It's because it's not as confusing to deal with a re-executing decision (on restart). According to Sec 10.8, Part 3d., the decider always re-executes on a restart.
So I think this could be fixed solely in the TCK.
But if it strikes people as interesting to be able to lift the restriction on re-execution of a decision, then maybe we put that on the table and add this to the SPEC column?
Let's address this by:
1. First fixing the TCK by shipping equivalent tests which use separate decisions so as not to loop. (The tests are important so I want to fix and retain them, not exclude).
2. Tagging as 'future' and consider lifting the restriction later.
Just to note, the following tests all also use an XML with a loop configured in it:
Sorry. I missed these two in my previous comment (they also are configured with a loop):
Reviewed the relevant tests (thanks for listing them Michael) and for each JSL was able to "break apart" the single decision into multiple, similar decisions.
Confirmed that there was no interesting test logic being exercised in having been using a single decision element previously.
In the test method source I went and added the comments explain the @assertion, @test_Strategy tags explaining what the tests are supposed to be testing (e.g. the fact that decider return value becomes job exit status, etc.).
I know there are quite a few methods missing these comments, though I only added them for now for these eight.