rondagostino commented on code in PR #13437:
URL: https://github.com/apache/kafka/pull/13437#discussion_r1173187845
##########
server-common/src/test/java/org/apache/kafka/server/immutable/pcollections/PCollectionsImmutableMapTest.java:
##########
@@ -225,77 +225,59 @@ public void testDelegationOfReplaceAll() {
new PCollectionsHashMapWrapperDelegationChecker<>()
.defineMockConfigurationForVoidMethodInvocation(mock ->
mock.replaceAll(eq(mockBiFunction)))
.defineWrapperVoidMethodInvocation(wrapper ->
wrapper.replaceAll(mockBiFunction))
- .doVoidMethodDelegationCheck();
+ .doUnsupportedVoidFunctionDelegrationCheck();
}
@Test
public void testDelegationOfPutIfAbsent() {
new PCollectionsHashMapWrapperDelegationChecker<>()
- .defineMockConfigurationForFunctionInvocation(mock ->
mock.putIfAbsent(eq(this), eq(this)), this)
-
.defineWrapperFunctionInvocationAndMockReturnValueTransformation(wrapper ->
wrapper.putIfAbsent(this, this), identity())
- .doFunctionDelegationCheck();
- }
-
- @ParameterizedTest
- @ValueSource(booleans = {true, false})
- public void testDelegationOfRemoveByKeyAndValue(boolean
mockFunctionReturnValue) {
- new PCollectionsHashMapWrapperDelegationChecker<>()
- .defineMockConfigurationForFunctionInvocation(mock ->
mock.remove(eq(this), eq(this)), mockFunctionReturnValue)
-
.defineWrapperFunctionInvocationAndMockReturnValueTransformation(wrapper ->
wrapper.remove(this, this), identity())
- .doFunctionDelegationCheck();
- }
-
- @ParameterizedTest
- @ValueSource(booleans = {true, false})
- public void testDelegationOfReplaceWhenMappedToSpecificValue(boolean
mockFunctionReturnValue) {
- new PCollectionsHashMapWrapperDelegationChecker<>()
- .defineMockConfigurationForFunctionInvocation(mock ->
mock.replace(eq(this), eq(this), eq(this)), mockFunctionReturnValue)
-
.defineWrapperFunctionInvocationAndMockReturnValueTransformation(wrapper ->
wrapper.replace(this, this, this), identity())
- .doFunctionDelegationCheck();
Review Comment:
Yeah, this is why I said in the comment above:
```
I think on the one hand we don't have to test that the real method throws
the exception -- that's up to the library itself to test in its test suite. All
we have to test is that we delegate correctly.
```
The underlying library implementation in I guess these two cases doesn't
explicitly throw the exception but instead reuses a default implementation from
`java.util`, and that will cause an exception to be thrown if the map ends up
needing to be mutated.
At a minimum we should put the two test cases back in as I wrote them. If
you want to do it with an explicit check for the thrown exception by invoking
the underlying method then you may have to do a bit more configuration on the
mock so that the underlying method will actually decide it needs to mutate the
map. probably just putting my stuff back in is easier (and also add
"UnsupportedOperation" into the test method names).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
