-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix console output problem, dotnet test appeared to hang #102
Fix console output problem, dotnet test appeared to hang #102
Conversation
|
||
namespace Akka.MultiNode.TestAdapter.Internal.Sinks | ||
{ | ||
public class FrameworkHandleMessageSinkActor: TestCoordinatorEnabledMessageSink |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New message sink for IFrameworkHandle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add an XML-DOC comment here to explain this?
} | ||
}; | ||
opt.OutputDataReceived = OutputHandler; | ||
opt.ErrorDataReceived = OutputHandler; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capture error outputs from test processes
@@ -49,8 +52,14 @@ public TcpLoggingServer(IActorRef sinkCoordinator) | |||
|
|||
Receive<Tcp.Received>(received => | |||
{ | |||
var message = received.Data.ToString(); | |||
sinkCoordinator.Tell(message); | |||
if (received.Data.Count >= BufferSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bug pertains to a lot of "unknown message" in our test. Although we don't have any limit on TCP message size, our buffer size is by default limited to 512 bytes, and since we pump messages as soon as we get them, if the string message is longer than 512 characters, the message will be chopped up. This happens a lot with FAIL-EXCEPTION messages since the stack trace can be quite big.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we increase the message size used by the MNTR in Akka.IO.Tcp?
@@ -112,15 +112,15 @@ public static MultiNodeTestRunnerMessageType DetermineMessageType(string message | |||
var matchRunnerLog = RunnerLogMessageRegex.Match(messageStr); | |||
if (matchRunnerLog.Success) return MultiNodeTestRunnerMessageType.RunnerLogMessage; | |||
|
|||
var matchFailureReason = NodeFailureReasonRegex.Match(messageStr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failure with exception need to be tested first, else the pass test will eat the exception messages because it contains the word "FAIL"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small changes
|
||
namespace Akka.MultiNode.TestAdapter.Internal.Sinks | ||
{ | ||
public class FrameworkHandleMessageSinkActor: TestCoordinatorEnabledMessageSink |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add an XML-DOC comment here to explain this?
@@ -49,8 +52,14 @@ public TcpLoggingServer(IActorRef sinkCoordinator) | |||
|
|||
Receive<Tcp.Received>(received => | |||
{ | |||
var message = received.Data.ToString(); | |||
sinkCoordinator.Tell(message); | |||
if (received.Data.Count >= BufferSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we increase the message size used by the MNTR in Akka.IO.Tcp?
Looks good overall but left some remarks |
Increasing the buffer size would not actually fix the problem, because we'd never know how big the stack trace will be, a very deep stack trace with lots of inner exceptions |
@Arkatufus yeah but 512 is exceptionally tiny. |
I'll bump it up to 10 KB then? that should be a fairly safe number |
Closes #100