ISPN-11050 StreamDistPartitionHandlingTest.testUsingIteratorButPartit… #8026
Conversation
|
This fixes failures in both testUsingIteratorButPartitionOccursAfterRetrievingRemoteValues and testUsingIteratorButPartitionOccursBeforeRetrievingRemoteValues. The former failed because the remote command may not be processed before the partition happened, thus it would still think it was parititioned (fixed by waiting before completion of publisher instead). The latter failed if the remote command completed before registering the whenComplete on its response thus running the response on the same thread, causing a deadlock (fixed by always waiting on a different thread). |
core/src/test/java/org/infinispan/partitionhandling/StreamDistPartitionHandlingTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/org/infinispan/partitionhandling/StreamDistPartitionHandlingTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/org/infinispan/partitionhandling/StreamDistPartitionHandlingTest.java
Outdated
Show resolved
Hide resolved
| */ | ||
| public static <C> C replaceComponentWithSpy(Cache<?,?> cache, Class<C> componentClass) { | ||
| C component = TestingUtil.extractComponent(cache, componentClass); | ||
| C spiedComponent = spy(component); |
There was a problem hiding this comment.
If at all possible, I'd rather create a mock that delegates to the existing component instead of using spy().
The instance created by spy() has all the annotations, so after copying the state of the existing component (without any synchronization), the component registry will call start() on the new instance, which may break things.
Of course, using mock() has the opposite problem: the component registry won't call stop() on it, which depending on component may hang the test during shutdown. For this reason I've usually written a custom mock class for the component I'm mocking, but it should be possible to use a generic AbstractMockComponent class that delegates injection/start/stop and then use mock(AbstractMockComponent.class, withSettings.additionalInterfaces(componentClass) to implement the needed interface.
And after writing all of this I realize it's probably too much for this PR, which is merely moving the method...
I'll put it on my TODO list and maybe get to it at some point :)
There was a problem hiding this comment.
I personally feel that our components should work fine being stopped and restarted, but that may be just a pipe dream.
There was a problem hiding this comment.
Maybe we could make components restart properly without removing them from the registry and adding a new instance, but that wouldn't really help with spy(), because you're copying a component that has already been started and you're starting it again without calling stop() before.
…ionOccursAfterRetrievingRemoteValues random failures
wburns
force-pushed
the
ISPN-11050_stream_part_handling_failures
branch
from
8c7e061
to
2df96f6
Compare
|
Updated. |
|
Merged, thanks Will! |
…ionOccursAfterRetrievingRemoteValues random failures
https://issues.redhat.com/browse/ISPN-11050