[fix] Fix data collection channels to use negotiated protocol version instead of V1

src/Microsoft.TestPlatform.CommunicationUtilities/DataCollectionRequestHandler.cs

@@ -48,6 +48,10 @@ internal class DataCollectionRequestHandler : IDataCollectionRequestHandler, IDi
     private readonly IFileHelper _fileHelper;
     private readonly IRequestData _requestData;
 
+    // The protocol version negotiated with the vstest.console sender.
+    // Set when BeforeTestRunStart is received; used for all responses on this channel.
+    private int _protocolVersion = 1;
+
     private Task? _testCaseEventMonitorTask;
 
     /// <summary>
@@ -212,7 +216,7 @@ public void ProcessRequests()
     /// </param>
     public void SendDataCollectionMessage(DataCollectionMessageEventArgs args)
     {
-        _communicationManager.SendMessage(MessageType.DataCollectionMessage, args);
+        _communicationManager.SendMessage(MessageType.DataCollectionMessage, args, _protocolVersion);
     }
 
     /// <summary>
@@ -296,6 +300,14 @@ private void AddExtensionAssemblies(BeforeTestRunStartPayload payload)
 
     private void HandleBeforeTestRunStart(Message message)
     {
+        // Negotiate the protocol version: adopt the highest version that both sides support.
+        // The sender transmits its highest supported version; we respond with the minimum of
+        // that and our own highest supported version so all subsequent messages use a mutually
+        // understood serialization format.
+        _protocolVersion = message.Version > 0
+            ? Math.Min(message.Version, ProtocolVersioning.HighestSupportedVersion)
+            : 1;
+
         // Initialize datacollectors and get environment variables.
         var payload = _dataSerializer.DeserializePayload<BeforeTestRunStartPayload>(message);
         TPDebug.Assert(payload is not null, "payload is null");
@@ -355,7 +367,8 @@ private void HandleBeforeTestRunStart(Message message)
 
         _communicationManager.SendMessage(
             MessageType.BeforeTestRunStartResult,
-            new BeforeTestRunStartResult(envVariables, testCaseEventsPort));
+            new BeforeTestRunStartResult(envVariables, testCaseEventsPort),
+            _protocolVersion);
 
         EqtTrace.Info("DataCollectionRequestHandler.ProcessRequests : DataCollection started.");
     }
@@ -395,7 +408,7 @@ private void HandleAfterTestRunEnd(Message message)
         // As datacollector process exits itself on parent process(vstest.console) exits.
         _dataCollectionManager?.Dispose();
 
-        _communicationManager.SendMessage(MessageType.AfterTestRunEndResult, afterTestRunEndResult);
+        _communicationManager.SendMessage(MessageType.AfterTestRunEndResult, afterTestRunEndResult, _protocolVersion);
         EqtTrace.Info("DataCollectionRequestHandler.ProcessRequests : Session End message received from server. Closing the connection.");
 
         Close();

src/Microsoft.TestPlatform.CommunicationUtilities/DataCollectionRequestSender.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Net;
@@ -25,6 +26,10 @@ public sealed class DataCollectionRequestSender : IDataCollectionRequestSender
     private readonly ICommunicationManager _communicationManager;
     private readonly IDataSerializer _dataSerializer;
 
+    // The protocol version negotiated with the datacollector.
+    // Set after SendBeforeTestRunStartAndGetResult reads the response version.
+    private int _protocolVersion = 1;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="DataCollectionRequestSender"/> class.
     /// </summary>
@@ -94,7 +99,7 @@ public void Close()
     /// <inheritdoc/>
     public void SendTestHostLaunched(TestHostLaunchedPayload testHostLaunchedPayload)
     {
-        _communicationManager.SendMessage(MessageType.TestHostLaunched, testHostLaunchedPayload);
+        _communicationManager.SendMessage(MessageType.TestHostLaunched, testHostLaunchedPayload, _protocolVersion);
     }
 
     /// <inheritdoc/>
@@ -112,7 +117,10 @@ public void SendTestHostLaunched(TestHostLaunchedPayload testHostLaunchedPayload
             IsTelemetryOptedIn = isTelemetryOptedIn
         };
 
-        _communicationManager.SendMessage(MessageType.BeforeTestRunStart, payload);
+        // Send at the highest version this side supports; the datacollector echoes back the
+        // highest version it supports in the BeforeTestRunStartResult response, which then
+        // becomes the negotiated version for all subsequent messages on this channel.
+        _communicationManager.SendMessage(MessageType.BeforeTestRunStart, payload, ProtocolVersioning.HighestSupportedVersion);
 
         while (!isDataCollectionStarted)
         {
@@ -133,6 +141,13 @@ public void SendTestHostLaunched(TestHostLaunchedPayload testHostLaunchedPayload
             else if (message.MessageType == MessageType.BeforeTestRunStartResult)
             {
                 isDataCollectionStarted = true;
+                // Adopt the version the datacollector used in the response as the negotiated
+                // protocol version for all subsequent messages on this channel.
+                if (message.Version > 0)
+                {
+                    _protocolVersion = Math.Min(message.Version, ProtocolVersioning.HighestSupportedVersion);
+                }
+
                 result = _dataSerializer.DeserializePayload<BeforeTestRunStartResult>(message);
             }
             else if (message.MessageType == MessageType.TelemetryEventMessage)
@@ -152,7 +167,7 @@ public void SendTestHostLaunched(TestHostLaunchedPayload testHostLaunchedPayload
 
         EqtTrace.Verbose("DataCollectionRequestSender.SendAfterTestRunStartAndGetResult: Send AfterTestRunEnd message with isCancelled: {0}", isCancelled);
 
-        _communicationManager.SendMessage(MessageType.AfterTestRunEnd, isCancelled);
+        _communicationManager.SendMessage(MessageType.AfterTestRunEnd, isCancelled, _protocolVersion);
 
         // Cycle through the messages that the datacollector sends.
         // Currently each of the operations are not separate tasks since they should not each take much time. This is just a notification.

src/Microsoft.TestPlatform.CommunicationUtilities/DataCollectionTestCaseEventHandler.cs

@@ -97,7 +97,7 @@ public void ProcessRequests()
                         EqtTrace.Error($"DataCollectionTestCaseEventHandler.ProcessRequests: Error occurred during TestCaseStarted event handling: {ex}");
                     }
 
-                    _communicationManager.SendMessage(MessageType.DataCollectionTestStartAck);
+                    _communicationManager.SendMessage(MessageType.DataCollectionTestStartAck, string.Empty, Math.Min(message.Version, ProtocolVersioning.HighestSupportedVersion));
 
                     EqtTrace.Info("DataCollectionTestCaseEventHandler: Test case '{0} - {1}' started.", testCaseStartEventArgs?.TestCaseName, testCaseStartEventArgs?.TestCaseId);
 
@@ -122,7 +122,7 @@ public void ProcessRequests()
                         attachmentSets = new Collection<AttachmentSet>();
                     }
 
-                    _communicationManager.SendMessage(MessageType.DataCollectionTestEndResult, attachmentSets);
+                    _communicationManager.SendMessage(MessageType.DataCollectionTestEndResult, attachmentSets, Math.Min(message.Version, ProtocolVersioning.HighestSupportedVersion));
 
                     EqtTrace.Info("DataCollectionTestCaseEventHandler: Test case '{0} - {1}' completed", testCaseEndEventArgs?.TestCaseName, testCaseEndEventArgs?.TestCaseId);
                     break;

src/Microsoft.TestPlatform.CommunicationUtilities/DataCollectionTestCaseEventSender.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
 using System.Collections.ObjectModel;
 using System.Net;
 
@@ -18,6 +19,10 @@ public class DataCollectionTestCaseEventSender : IDataCollectionTestCaseEventSen
     private readonly ICommunicationManager _communicationManager;
     private readonly IDataSerializer _dataSerializer;
 
+    // Protocol version negotiated with the datacollector test case event handler.
+    // Updated from the DataCollectionTestStartAck echo after the first SendTestCaseStart.
+    private int _protocolVersion = ProtocolVersioning.HighestSupportedVersion;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="DataCollectionTestCaseEventSender"/> class.
     /// </summary>
@@ -82,20 +87,27 @@ public void Close()
     /// <inheritdoc />
     public void SendTestCaseStart(TestCaseStartEventArgs e)
     {
-        _communicationManager.SendMessage(MessageType.DataCollectionTestStart, e);
+        _communicationManager.SendMessage(MessageType.DataCollectionTestStart, e, _protocolVersion);
 
         var message = _communicationManager.ReceiveMessage();
         if (message != null && message.MessageType != MessageType.DataCollectionTestStartAck)
         {
             EqtTrace.Error("DataCollectionTestCaseEventSender.SendTestCaseStart : MessageType.DataCollectionTestStartAck not received.");
         }
+
+        // Adopt the version echoed by the handler as the negotiated protocol version for all
+        // subsequent sends on this sub-channel.
+        if (message?.Version > 0)
+        {
+            _protocolVersion = Math.Min(message.Version, ProtocolVersioning.HighestSupportedVersion);
+        }
     }
 
     /// <inheritdoc />
     public Collection<AttachmentSet>? SendTestCaseEnd(TestCaseEndEventArgs e)
     {
         var attachmentSets = new Collection<AttachmentSet>();
-        _communicationManager.SendMessage(MessageType.DataCollectionTestEnd, e);
+        _communicationManager.SendMessage(MessageType.DataCollectionTestEnd, e, _protocolVersion);
 
         var message = _communicationManager.ReceiveMessage();
         if (message != null && message.MessageType == MessageType.DataCollectionTestEndResult)
@@ -109,6 +121,6 @@ public void SendTestCaseStart(TestCaseStartEventArgs e)
     /// <inheritdoc />
     public void SendTestSessionEnd(SessionEndEventArgs e)
     {
-        _communicationManager.SendMessage(MessageType.SessionEnd, e);
+        _communicationManager.SendMessage(MessageType.SessionEnd, e, _protocolVersion);
     }
 }

test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/DataCollectionRequestHandlerTests.cs

@@ -127,13 +127,13 @@ public void SendDataCollectionMessageShouldSendMessageToCommunicationManager()
 
         _requestHandler.SendDataCollectionMessage(message);
 
-        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.DataCollectionMessage, message), Times.Once);
+        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.DataCollectionMessage, message, It.IsAny<int>()), Times.Once);
     }
 
     [TestMethod]
     public void SendDataCollectionMessageShouldThrowExceptionIfThrownByCommunicationManager()
     {
-        _mockCommunicationManager.Setup(x => x.SendMessage(MessageType.DataCollectionMessage, It.IsAny<DataCollectionMessageEventArgs>())).Throws<Exception>();
+        _mockCommunicationManager.Setup(x => x.SendMessage(MessageType.DataCollectionMessage, It.IsAny<DataCollectionMessageEventArgs>(), It.IsAny<int>())).Throws<Exception>();
         var message = new DataCollectionMessageEventArgs(TestMessageLevel.Error, "message");
 
         Assert.ThrowsExactly<Exception>(() => _requestHandler.SendDataCollectionMessage(message));
@@ -163,6 +163,30 @@ public void DisposeShouldCloseCommunicationChannel()
         _mockCommunicationManager.Verify(x => x.StopClient(), Times.Once);
     }
 
+    [TestMethod]
+    public void ProcessRequestsShouldNegotiateProtocolVersionToMinOfRequestAndHighest()
+    {
+        // Simulate a sender that supports only version 4 (less than HighestSupportedVersion = 7).
+        var beforeTestRunStartAtV4 = new Message()
+        {
+            MessageType = MessageType.BeforeTestRunStart,
+            Version = 4,
+            RawMessage = JsonDataSerializer.Instance.SerializePayload(MessageType.BeforeTestRunStart, new BeforeTestRunStartPayload { SettingsXml = "settingsxml", Sources = new List<string> { "test1.dll" } }, 4)
+        };
+
+        _mockCommunicationManager.SetupSequence(x => x.ReceiveMessage()).Returns(beforeTestRunStartAtV4).Returns(_afterTestRunEnd);
+        _mockDataCollectionManager.Setup(x => x.SessionStarted(It.IsAny<SessionStartEventArgs>())).Returns(true);
+        var payload = new BeforeTestRunStartPayload { SettingsXml = "settingsxml", Sources = new List<string> { "test1.dll" } };
+        _mockDataSerializer.Setup(x => x.DeserializePayload<BeforeTestRunStartPayload>(It.Is<Message>(y => y.MessageType == MessageType.BeforeTestRunStart)))
+            .Returns(payload);
+
+        _requestHandler.ProcessRequests();
+
+        // Negotiated version = Math.Min(4, HighestSupportedVersion=7) = 4; all responses must use it.
+        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.BeforeTestRunStartResult, It.IsAny<BeforeTestRunStartResult>(), 4), Times.Once);
+        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.AfterTestRunEndResult, It.IsAny<AfterTestRunEndResult>(), 4), Times.Once);
+    }
+
     [TestMethod]
     public void ProcessRequestsShouldProcessRequests()
     {
@@ -189,14 +213,14 @@ public void ProcessRequestsShouldProcessRequests()
 
         // Verify SessionStarted events
         _mockDataCollectionManager.Verify(x => x.SessionStarted(It.IsAny<SessionStartEventArgs>()), Times.Once);
-        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.BeforeTestRunStartResult, It.IsAny<BeforeTestRunStartResult>()), Times.Once);
+        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.BeforeTestRunStartResult, It.IsAny<BeforeTestRunStartResult>(), It.IsAny<int>()), Times.Once);
 
         // Verify TestHostLaunched events
         _mockDataCollectionManager.Verify(x => x.TestHostLaunched(1234), Times.Once);
 
         // Verify AfterTestRun events.
         _mockDataCollectionManager.Verify(x => x.SessionEnded(It.IsAny<bool>()), Times.Once);
-        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.AfterTestRunEndResult, It.IsAny<AfterTestRunEndResult>()), Times.Once);
+        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.AfterTestRunEndResult, It.IsAny<AfterTestRunEndResult>(), It.IsAny<int>()), Times.Once);
     }
 
     [TestMethod]

test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/DataCollectionRequestSenderTests.cs

@@ -124,7 +124,7 @@ public void SendBeforeTestRunStartAndGetResultShouldSendBeforeTestRunStartMessag
         _mockDataSerializer.Setup(x => x.DeserializeMessage(rawMessage)).Returns(new Message() { MessageType = MessageType.BeforeTestRunStartResult });
         _requestSender.SendBeforeTestRunStartAndGetResult(string.Empty, testSources, true, null);
 
-        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.BeforeTestRunStart, It.Is<BeforeTestRunStartPayload>(p => p.SettingsXml == string.Empty && p.IsTelemetryOptedIn)));
+        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.BeforeTestRunStart, It.Is<BeforeTestRunStartPayload>(p => p.SettingsXml == string.Empty && p.IsTelemetryOptedIn), ProtocolVersioning.HighestSupportedVersion));
     }
 
     [TestMethod]
@@ -140,7 +140,7 @@ public void SendBeforeTestRunStartAndGetResultShouldSendRawMessageIfTelemetry()
         _requestSender.SendBeforeTestRunStartAndGetResult(string.Empty, testSources, true, handlerMock.Object);
 
         handlerMock.Verify(x => x.HandleRawMessage(rawMessage1));
-        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.BeforeTestRunStart, It.Is<BeforeTestRunStartPayload>(p => p.SettingsXml == string.Empty && p.IsTelemetryOptedIn)));
+        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.BeforeTestRunStart, It.Is<BeforeTestRunStartPayload>(p => p.SettingsXml == string.Empty && p.IsTelemetryOptedIn), ProtocolVersioning.HighestSupportedVersion));
     }
 
     [TestMethod]
@@ -154,6 +154,6 @@ public void SendBeforeTestRunStartAndGetResultShouldNotSendRawMessageIfTelemetry
         _mockDataSerializer.Setup(x => x.DeserializeMessage(rawMessage2)).Returns(new Message() { MessageType = MessageType.BeforeTestRunStartResult });
         _requestSender.SendBeforeTestRunStartAndGetResult(string.Empty, testSources, true, null);
 
-        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.BeforeTestRunStart, It.Is<BeforeTestRunStartPayload>(p => p.SettingsXml == string.Empty && p.IsTelemetryOptedIn)));
+        _mockCommunicationManager.Verify(x => x.SendMessage(MessageType.BeforeTestRunStart, It.Is<BeforeTestRunStartPayload>(p => p.SettingsXml == string.Empty && p.IsTelemetryOptedIn), ProtocolVersioning.HighestSupportedVersion));
     }
 }