Bug 5412 - DeciderTests#testDeciderExitStatusIsSetOnJobContext and step looping
DeciderTests#testDeciderExitStatusIsSetOnJobContext and step looping
Product: jbatch
Classification: Unclassified
Component: TCK
All All
: P5 normal
: ---
Assigned To: ScottKurz
Depends on:
  Show dependency treegraph
Reported: 2013-09-24 14:33 UTC by mminella
Modified: 2014-01-03 03:47 UTC (History)
1 user (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description mminella 2013-09-24 14:33:26 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.
Comment 1 ScottKurz 2013-09-24 16:03:34 UTC
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?
Comment 2 ScottKurz 2013-11-09 19:09:15 UTC
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.
Comment 3 mminella 2013-12-17 17:21:14 UTC
Just to note, the following tests all also use an XML with a loop configured in it:
* DeciderTests#testDeciderTransitionFromFlowAndAllowRestart
* DeciderTests#testDeciderTransitionFromFlowAndAllowRestartFalse
* DeciderTests#testDeciderTransitionFromStepAndAllowRestart
* DeciderTests#testDeciderTransitionFromStepAndAllowRestartFalse
* DeciderTests#testDeciderTransitionFromStepWithinFlowAndAllowRestart
* DeciderTests#testDeciderTransitionFromStepWithinFlowAndAllowRestartFalse
Comment 4 mminella 2013-12-17 17:26:35 UTC
Sorry.  I missed these two in my previous comment (they also are configured with a loop):

* DeciderTests#testDeciderTransitionFromSplitAndAllowRestart
* DeciderTests#testDeciderTransitionFromSplitAndAllowRestartFalse
Comment 5 ScottKurz 2014-01-03 03:47:20 UTC
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.