From 252eb4371f82ac196b2d5680e5fafbcf578a638e Mon Sep 17 00:00:00 2001 From: Livar Cunha Date: Tue, 23 Feb 2016 18:13:00 -0800 Subject: [PATCH 1/2] Making the version check an optional message for test discovery and test run. --- ...tTestRunnerProcessStartInfoMessageHandler.cs | 8 +++++++- .../TestDiscoveryStartMessageHandler.cs | 9 +++++++-- .../GivenATestDiscoveryStartMessageHandler.cs | 16 ++++++++++++++-- ...tTestRunnerProcessStartInfoMessageHandler.cs | 17 +++++++++++++++-- 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/dotnet/commands/dotnet-test/MessageHandlers/GetTestRunnerProcessStartInfoMessageHandler.cs b/src/dotnet/commands/dotnet-test/MessageHandlers/GetTestRunnerProcessStartInfoMessageHandler.cs index a95cc3a06..aa7ed47a6 100644 --- a/src/dotnet/commands/dotnet-test/MessageHandlers/GetTestRunnerProcessStartInfoMessageHandler.cs +++ b/src/dotnet/commands/dotnet-test/MessageHandlers/GetTestRunnerProcessStartInfoMessageHandler.cs @@ -55,8 +55,14 @@ namespace Microsoft.DotNet.Tools.Test private static bool CanHandleMessage(IDotnetTest dotnetTest, Message message) { - return dotnetTest.State == DotnetTestState.VersionCheckCompleted && + return IsAtAnAcceptableState(dotnetTest) && message.MessageType == TestMessageTypes.TestExecutionGetTestRunnerProcessStartInfo; } + + private static bool IsAtAnAcceptableState(IDotnetTest dotnetTest) + { + return dotnetTest.State == DotnetTestState.VersionCheckCompleted || + dotnetTest.State == DotnetTestState.InitialState; + } } } diff --git a/src/dotnet/commands/dotnet-test/MessageHandlers/TestDiscoveryStartMessageHandler.cs b/src/dotnet/commands/dotnet-test/MessageHandlers/TestDiscoveryStartMessageHandler.cs index f4088f2d6..a791ee776 100644 --- a/src/dotnet/commands/dotnet-test/MessageHandlers/TestDiscoveryStartMessageHandler.cs +++ b/src/dotnet/commands/dotnet-test/MessageHandlers/TestDiscoveryStartMessageHandler.cs @@ -68,8 +68,13 @@ namespace Microsoft.DotNet.Cli.Tools.Test private static bool CanHandleMessage(IDotnetTest dotnetTest, Message message) { - return dotnetTest.State == DotnetTestState.VersionCheckCompleted && - message.MessageType == TestMessageTypes.TestDiscoveryStart; + return IsAtAnAcceptableState(dotnetTest) && message.MessageType == TestMessageTypes.TestDiscoveryStart; + } + + private static bool IsAtAnAcceptableState(IDotnetTest dotnetTest) + { + return (dotnetTest.State == DotnetTestState.VersionCheckCompleted || + dotnetTest.State == DotnetTestState.InitialState); } } } diff --git a/test/dotnet-test.UnitTests/GivenATestDiscoveryStartMessageHandler.cs b/test/dotnet-test.UnitTests/GivenATestDiscoveryStartMessageHandler.cs index 36cedfe36..c82effcf9 100644 --- a/test/dotnet-test.UnitTests/GivenATestDiscoveryStartMessageHandler.cs +++ b/test/dotnet-test.UnitTests/GivenATestDiscoveryStartMessageHandler.cs @@ -61,7 +61,7 @@ namespace Microsoft.Dotnet.Tools.Test.Tests } [Fact] - public void It_returns_NoOp_if_the_dotnet_test_state_is_not_VersionCheckCompleted() + public void It_returns_NoOp_if_the_dotnet_test_state_is_not_VersionCheckCompleted_or_InitialState() { var dotnetTestMock = new Mock(); dotnetTestMock.Setup(d => d.State).Returns(DotnetTestState.Terminated); @@ -84,7 +84,19 @@ namespace Microsoft.Dotnet.Tools.Test.Tests } [Fact] - public void It_returns_TestDiscoveryCompleted_when_it_handles_the_message() + public void It_returns_TestDiscoveryCompleted_when_it_handles_the_message_and_current_state_is_InitialState() + { + var dotnetTestMock = new Mock(); + dotnetTestMock.Setup(d => d.State).Returns(DotnetTestState.InitialState); + + var nextState = + _testDiscoveryStartMessageHandler.HandleMessage(dotnetTestMock.Object, _validMessage); + + nextState.Should().Be(DotnetTestState.TestDiscoveryStarted); + } + + [Fact] + public void It_returns_TestDiscoveryCompleted_when_it_handles_the_message_and_current_state_is_VersionCheckCompleted() { var nextState = _testDiscoveryStartMessageHandler.HandleMessage(_dotnetTestAtVersionCheckCompletedState, _validMessage); diff --git a/test/dotnet-test.UnitTests/GivenATestExecutionGetTestRunnerProcessStartInfoMessageHandler.cs b/test/dotnet-test.UnitTests/GivenATestExecutionGetTestRunnerProcessStartInfoMessageHandler.cs index be0f7555b..cd14ab1c9 100644 --- a/test/dotnet-test.UnitTests/GivenATestExecutionGetTestRunnerProcessStartInfoMessageHandler.cs +++ b/test/dotnet-test.UnitTests/GivenATestExecutionGetTestRunnerProcessStartInfoMessageHandler.cs @@ -68,7 +68,7 @@ namespace Microsoft.Dotnet.Tools.Test.Tests } [Fact] - public void It_returns_NoOp_if_the_dotnet_test_state_is_not_VersionCheckCompleted() + public void It_returns_NoOp_if_the_dotnet_test_state_is_not_VersionCheckCompleted_or_InitialState() { var dotnetTestMock = new Mock(); dotnetTestMock.Setup(d => d.State).Returns(DotnetTestState.Terminated); @@ -91,7 +91,20 @@ namespace Microsoft.Dotnet.Tools.Test.Tests } [Fact] - public void It_returns_TestExecutionSentTestRunnerProcessStartInfo_when_it_handles_the_message() + public void It_returns_TestExecutionSentTestRunnerProcessStartInfo_when_it_handles_the_message_and_current_state_is_InitialState() + { + var dotnetTestMock = new Mock(); + dotnetTestMock.Setup(d => d.State).Returns(DotnetTestState.InitialState); + + var nextState = _testGetTestRunnerProcessStartInfoMessageHandler.HandleMessage( + dotnetTestMock.Object, + _validMessage); + + nextState.Should().Be(DotnetTestState.TestExecutionSentTestRunnerProcessStartInfo); + } + + [Fact] + public void It_returns_TestExecutionSentTestRunnerProcessStartInfo_when_it_handles_the_message_and_current_state_is_VersionCheckCompleted() { var nextState = _testGetTestRunnerProcessStartInfoMessageHandler.HandleMessage( _dotnetTestMock.Object, From 3f2b1d068d0253c90f410b88c2c66a4d1f32295f Mon Sep 17 00:00:00 2001 From: Livar Cunha Date: Wed, 24 Feb 2016 16:53:16 -0800 Subject: [PATCH 2/2] Making the reporing channel port discovery and accept incoming connections separate. Before we were hanging when creating the reporting channel and never starting the test runner. --- .../commands/dotnet-test/IReportingChannel.cs | 3 ++ ...estRunnerProcessStartInfoMessageHandler.cs | 2 + .../TestDiscoveryStartMessageHandler.cs | 2 + src/dotnet/commands/dotnet-test/Program.cs | 2 + .../commands/dotnet-test/ReportingChannel.cs | 52 ++++++++++++------- .../GivenATestDiscoveryStartMessageHandler.cs | 10 ++++ ...estRunnerProcessStartInfoMessageHandler.cs | 10 ++++ 7 files changed, 61 insertions(+), 20 deletions(-) diff --git a/src/dotnet/commands/dotnet-test/IReportingChannel.cs b/src/dotnet/commands/dotnet-test/IReportingChannel.cs index 64fa1d6f7..f103c61e2 100644 --- a/src/dotnet/commands/dotnet-test/IReportingChannel.cs +++ b/src/dotnet/commands/dotnet-test/IReportingChannel.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Threading.Tasks; using Microsoft.Extensions.Testing.Abstractions; namespace Microsoft.DotNet.Tools.Test @@ -12,6 +13,8 @@ namespace Microsoft.DotNet.Tools.Test int Port { get; } + void Accept(); + void Send(Message message); void SendError(string error); diff --git a/src/dotnet/commands/dotnet-test/MessageHandlers/GetTestRunnerProcessStartInfoMessageHandler.cs b/src/dotnet/commands/dotnet-test/MessageHandlers/GetTestRunnerProcessStartInfoMessageHandler.cs index aa7ed47a6..17ed2092f 100644 --- a/src/dotnet/commands/dotnet-test/MessageHandlers/GetTestRunnerProcessStartInfoMessageHandler.cs +++ b/src/dotnet/commands/dotnet-test/MessageHandlers/GetTestRunnerProcessStartInfoMessageHandler.cs @@ -41,6 +41,8 @@ namespace Microsoft.DotNet.Tools.Test dotnetTest.StartListeningTo(testRunnerChannel); + testRunnerChannel.Accept(); + var testRunner = _testRunnerFactory.CreateTestRunner( new RunTestsArgumentsBuilder(dotnetTest.PathToAssemblyUnderTest, testRunnerChannel.Port, message)); diff --git a/src/dotnet/commands/dotnet-test/MessageHandlers/TestDiscoveryStartMessageHandler.cs b/src/dotnet/commands/dotnet-test/MessageHandlers/TestDiscoveryStartMessageHandler.cs index a791ee776..c6dcef258 100644 --- a/src/dotnet/commands/dotnet-test/MessageHandlers/TestDiscoveryStartMessageHandler.cs +++ b/src/dotnet/commands/dotnet-test/MessageHandlers/TestDiscoveryStartMessageHandler.cs @@ -55,6 +55,8 @@ namespace Microsoft.DotNet.Cli.Tools.Test dotnetTest.StartListeningTo(testRunnerChannel); + testRunnerChannel.Accept(); + var testRunner = _testRunnerFactory.CreateTestRunner( new DiscoverTestsArgumentsBuilder(dotnetTest.PathToAssemblyUnderTest, testRunnerChannel.Port)); diff --git a/src/dotnet/commands/dotnet-test/Program.cs b/src/dotnet/commands/dotnet-test/Program.cs index 9168314e2..6ac79ccb0 100644 --- a/src/dotnet/commands/dotnet-test/Program.cs +++ b/src/dotnet/commands/dotnet-test/Program.cs @@ -141,6 +141,8 @@ namespace Microsoft.DotNet.Tools.Test dotnetTest.StartListeningTo(adapterChannel); + adapterChannel.Accept(); + dotnetTest.StartHandlingMessages(); } } diff --git a/src/dotnet/commands/dotnet-test/ReportingChannel.cs b/src/dotnet/commands/dotnet-test/ReportingChannel.cs index b1663c994..2b9468bc7 100644 --- a/src/dotnet/commands/dotnet-test/ReportingChannel.cs +++ b/src/dotnet/commands/dotnet-test/ReportingChannel.cs @@ -7,6 +7,7 @@ using System.IO; using System.Net; using System.Net.Sockets; using System.Threading; +using System.Threading.Tasks; using Microsoft.Extensions.Testing.Abstractions; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -18,37 +19,47 @@ namespace Microsoft.DotNet.Tools.Test public static ReportingChannel ListenOn(int port) { // This fixes the mono incompatibility but ties it to ipv4 connections - using (var listenSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)) - { - listenSocket.Bind(new IPEndPoint(IPAddress.Loopback, port)); - listenSocket.Listen(10); + var listenSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - var socket = listenSocket.Accept(); + listenSocket.Bind(new IPEndPoint(IPAddress.Loopback, port)); + listenSocket.Listen(10); - return new ReportingChannel(socket); - } + return new ReportingChannel(listenSocket); } - private readonly BinaryWriter _writer; - private readonly BinaryReader _reader; + private BinaryWriter _writer; + private BinaryReader _reader; + private Socket _listenSocket; - private ReportingChannel(Socket socket) + private ReportingChannel(Socket listenSocket) { - Socket = socket; - - var stream = new NetworkStream(Socket); - _writer = new BinaryWriter(stream); - _reader = new BinaryReader(stream); - - // Read incoming messages on the background thread - new Thread(ReadMessages) { IsBackground = true }.Start(); + _listenSocket = listenSocket; + Port = ((IPEndPoint)listenSocket.LocalEndPoint).Port; } public event EventHandler MessageReceived; public Socket Socket { get; private set; } - public int Port => ((IPEndPoint) Socket.LocalEndPoint).Port; + public int Port { get; } + + public void Accept() + { + new Thread(() => + { + using (_listenSocket) + { + Socket = _listenSocket.Accept(); + + var stream = new NetworkStream(Socket); + _writer = new BinaryWriter(stream); + _reader = new BinaryReader(stream); + + // Read incoming messages on the background thread + new Thread(ReadMessages) { IsBackground = true }.Start(); + } + }) { IsBackground = true }.Start(); + } public void Send(Message message) { @@ -99,7 +110,8 @@ namespace Microsoft.DotNet.Tools.Test { try { - var message = JsonConvert.DeserializeObject(_reader.ReadString()); + var rawMessage = _reader.ReadString(); + var message = JsonConvert.DeserializeObject(rawMessage); MessageReceived?.Invoke(this, message); } diff --git a/test/dotnet-test.UnitTests/GivenATestDiscoveryStartMessageHandler.cs b/test/dotnet-test.UnitTests/GivenATestDiscoveryStartMessageHandler.cs index c82effcf9..d70242130 100644 --- a/test/dotnet-test.UnitTests/GivenATestDiscoveryStartMessageHandler.cs +++ b/test/dotnet-test.UnitTests/GivenATestDiscoveryStartMessageHandler.cs @@ -134,6 +134,16 @@ namespace Microsoft.Dotnet.Tools.Test.Tests _reportingChannelFactoryMock.Verify(r => r.CreateChannelWithAnyAvailablePort(), Times.Once); } + [Fact] + public void It_calls_accept_on_the_test_runner_channel() + { + _testDiscoveryStartMessageHandler.HandleMessage( + _dotnetTestMock.Object, + _validMessage); + + _testRunnerChannelMock.Verify(t => t.Accept(), Times.Once); + } + [Fact] public void It_makes_dotnet_test_listen_on_the_test_runner_port_for_messages_when_it_handles_the_message() { diff --git a/test/dotnet-test.UnitTests/GivenATestExecutionGetTestRunnerProcessStartInfoMessageHandler.cs b/test/dotnet-test.UnitTests/GivenATestExecutionGetTestRunnerProcessStartInfoMessageHandler.cs index cd14ab1c9..f1e4d1b61 100644 --- a/test/dotnet-test.UnitTests/GivenATestExecutionGetTestRunnerProcessStartInfoMessageHandler.cs +++ b/test/dotnet-test.UnitTests/GivenATestExecutionGetTestRunnerProcessStartInfoMessageHandler.cs @@ -148,6 +148,16 @@ namespace Microsoft.Dotnet.Tools.Test.Tests _reportingChannelFactoryMock.Verify(r => r.CreateChannelWithAnyAvailablePort(), Times.Once); } + [Fact] + public void It_calls_accept_on_the_test_runner_channel() + { + _testGetTestRunnerProcessStartInfoMessageHandler.HandleMessage( + _dotnetTestMock.Object, + _validMessage); + + _testRunnerChannelMock.Verify(t => t.Accept(), Times.Once); + } + [Fact] public void It_makes_dotnet_test_listen_on_the_test_runner_port_for_messages_when_it_handles_the_message() {