[GitHub] [maven-surefire] kriegaex edited a comment on pull request #343: SUREFIRE-1881 Adds additional debug log and fork connection timeout

[GitBox <gi...@apache.org>]

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.
   
   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:
users@infra.apache.org