kriegaex edited a comment on pull request #343:
URL: https://github.com/apache/maven-surefire/pull/343#issuecomment-836107707
To me it looks as if the whole `SurefireForkChannel.AcceptanceHandler`
implementation and usage are not done yet, which is why
`E2ETest::shouldNotVerifyClient` fails:
* The handler's `complete` method is never called and even if the test
would do it, the `completed` method would not be called.
* Because the result is only exposed as `Completable` and not as
`CompletionHandler`, the test could not call it either.
* Even if the `completed` method was called from inside
`SurefireForkChannel::connectToClient`, the test would have no way of accessing
`SurefireForkChannel.AcceptanceHandler.messageOfInvalidSessionIdException`,
because the state is not exposed.
* The result is that `InvalidSessionException` is never thrown.
Sorry, I explained a lot in one paragraph, but you know the code better than
me and probably understand what I mean. So this is unfinished work. Can you
explain why started refactoring the code to use the completion handler within
the confines of this bugfix? I am not seeing (yet), what you are aiming at,
i.e. which benefit it is meant to provide.
As for the test timing out, I did not inspect it yet. First I wanted to see
why the other test fails.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
