SSLEngine.unwrap precondition tests.#1414
Conversation
This is completely analagous to google#1412 but for unwrap(). Does not include failing tests for google#1372.
| () -> newConnectedEngine().unwrap(buffer, buffers, 0, arrayLength + 1)); | ||
| assertThrows(IndexOutOfBoundsException.class, | ||
| () -> newConnectedEngine().unwrap(buffer, buffers, arrayLength, 1)); | ||
| wrapThenUnwrap(bufferSize, buffers, 0, arrayLength); |
There was a problem hiding this comment.
I'd prefer to move wrapThenUnwrap into a new test.
There was a problem hiding this comment.
Thanks for the review!
Just for clarity, you're basically suggesting to split into a precondition failures test and and precondition "should pass" test? I can do that and add suitable comments.
There was a problem hiding this comment.
yes, exactly. it is also related to the other comment: for the prcondition pass test, can we re-use the input data? or should we create new input data for each test case, to make sure that the previous test doesn't change the test data?
There was a problem hiding this comment.
The pass cases as they currently are, will often return an error due to data re-use, under/overflow etc but not throw. There are tests elsewhere to check data flows correctly. These tests are just to ensure that the preconditions checks performed by warp/unwrap behave as expected.
That said, it's probably good practice to ensure that pass tests work end to end, so I'll split the wrap ones out too.
| } | ||
|
|
||
| @Test | ||
| public void unwrapPreconditions() throws Exception { |
There was a problem hiding this comment.
This test first sets up some test data, and then calls unwrap several times. This only works as expected if the test data is not changed, which is not guaranteed for unwrap. It should be fine here, because the precondition check should fail at the start before any changes to the input happen. But still, we should make sure that the reader is aware of this. So I think this should either create fresh input data for each call, or have a comment on why it is fine to reuse the input data.
There was a problem hiding this comment.
Good call, I'll rework it.
2 participants
This is completely analagous to #1412 but for unwrap().
Does not include failing tests for #1372.