-
Notifications
You must be signed in to change notification settings - Fork 9
Added test for Issue #18 that verifies each partition gets its own PUD #63
Conversation
}, | ||
apiRefs = { @APIRef(className="javax.batch.runtime.context.StepContext", methodNames={"setPersistentUserData", "getPersistentUserData"}) }, | ||
issueRefs = {"https://github.com/WASdev/standards.jsr352.tck/issues/18"}, | ||
strategy = "See comments in the code for this test" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious.. did you just thing it reads better visually or was it all the backslashing that steered you to doing it this way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the last discussion we had about the annotations was that we should make them smaller, and that the test strategy aka implementation details don't need to show up in the TCK test coverage report
|
||
PUDString = "PUD for Partition: " +partitionNumber; | ||
stepCtx.setPersistentUserData(PUDString); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initializing the PUDString in open() assures partitionNumber will have been injected. But if you're going to set it on the restart too it renders the later check not as interesting. You probably want to guard the set with if (checkpoint==null) {
} | ||
} | ||
|
||
//Code below is take from BasicReader |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe encapsulate all in another method... not essential, just a thought.
if (checkpoint == null) { | ||
stepCtx.setPersistentUserData(PUDString); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this since if you're going to set it every time doesn't it render the check in readItem() less interesting in exec #2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a dup of the previous comment which I thought had been lost.
No description provided.