emissionnebula commented on code in PR #13437:
URL: https://github.com/apache/kafka/pull/13437#discussion_r1172848492
##########
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:
These two functions are not overridden by PCollection classes.
`mock.replace()` or `mock.remove()` will call Java's default implementation in
`Map.replace()` or `AbstractMap.remove()`. Mocking that testing that seemed not
required.
For example: `Map.replace()` looks like this:
```
default boolean replace(K key, V oldValue, V newValue) {
Object curValue = get(key);
if (!Objects.equals(curValue, oldValue) ||
(curValue == null && !containsKey(key))) {
return false;
}
put(key, newValue);
return true;
}
```
Since it internally calls `put()` which we are already testing, so that's
why I thought it was not necessary to test these functions. Likewise for
`remove()`.
--
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
