Bug 4706

Summary: TCK ExecutionTests issues
Product: jbatch Reporter: mminella
Component: sourceAssignee: ScottKurz
Severity: normal CC: cvignola, issues
Priority: P5    
Version: 1   
Target Milestone: ---   
Hardware: All   
OS: All   

Description mminella 2013-02-23 02:39:51 UTC
1.  testInvokeJobWithOneBatchletStep, testInvokeJobWithTwoStepSequenceOfBatchlets, testInvokeJobWithFourStepSequenceOfBatchlets - Does not validate exit status (which would validate that the batchlet actually ran) of the job or the steps.
2.  testInvokeJobWithNextElement - Does not validate exit status or that the second step ran (job is flagged complete if either step2 runs to completion or step1 returns an exit status of ENDED as configured).
3.  testInvokeJobWithStopElement - Does not validate that the job stopped after the first step.
4.  testInvokeJobSimpleChunk - Does not validate that any chunk related processing was called (were reader/processor/writers actually called).
5.  testCheckpoint - How does this test verify checkpointing functionality?  Also, why does the ItemWriter in this test rely on the context's persistentUserData to store a good amount of the job's state (instead of relying on the checkpoint)?
6.  testSimpleFlow - job_flow_batchlet_4steps.xml…the step2 should not have a next element configured.
Comment 1 ScottKurz 2013-03-18 06:27:05 UTC
We did address point 6. which was an issue in a number of JSLs.  

Re: 5.  There is a story here.

You've probably seen a lot of the pattern where the artifact is somehow parameterized by an "execution number" directing the batch artfact to blow up on execution 1, then complete on execution 2.

This allows us to do restart and some associated things (verifying we persist snd rehydrate checkpoint correcdtly, etc.).

For awhile we were passing in this execution # as a job param on start() with one value e.g. "1" and then overriding on restart() with a different value e.g. "2"

When restart properties went away for a time, we jumped on the persistent user data as a way to "know" which execution we were in.  We had the artifact "increment" a counter in persistent user data on each execution.

It's less natural to do that with checkpoint since you typically will see the same "cursor"/checkpoint values across both executions.

Anyway we still use persistent data in some tests..and in others we're back to using restart job param.

I realize I didn't address the other 4 comments...
Comment 2 mminella 2013-03-18 16:03:43 UTC
In response to Comment #1:

I'm not sure that explains my question.  ExecutionTests#testCheckpoint doesn't do anything explicitly with the checkpoints.  There is no validation that they were taken or used in any way (unless I'm missing something).  It runs the job once and the parameters passed in do not trigger any of the interesting conditions the job may run.  Finally, the writer seems to use the stepContext's persistent user data as a way to maintain the state of the writer.  Since this is really the responsibility of the checkpoint data, I think it's a bad idea to implement this test this way since it is confusing for anyone looking to the TCK as a form of documentation.
Comment 3 ScottKurz 2013-03-18 16:32:42 UTC

I agree I didn't fully respond to your point.

I was just using it as an opportunity to explain a coding practice that might look particularly confusing... like I mentioned, it's partly to handle the period where we didn't have the ability to change restart parms, and we still employ this mechanism on other tests.

I didn't address your comment about checkpointing, nor #1-4 either...
Comment 4 cvignola 2013-08-29 14:25:08 UTC
It looks like all of these could be considered enhancements.  There are no questions being raised about the spec behavior.  The comments are saying that the TCK should do a better job covering the spec assertions, but not claiming any incompatibility with the spec or flat out incorrect behavior.

This doesn't seem helpful to keep open as a "bug", since we don't plan on updating the TCK in this way, (with new and tightened assertions).

However, if this information seems helpful to TCK consumers to better understand what the TCK does and doesn't enforce, we could maintain a reference to this type of information in a Wiki page.