Skip to content

Conversation

@Evangelink
Copy link
Member

Fixes #6351

namespace Microsoft.Testing.Platform.IPC.Models;

internal sealed record CommandLineOptionMessage(string? Name, string? Description, bool? IsHidden, bool? IsBuiltIn);
internal sealed record CommandLineOptionMessage(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we missing the attribute here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what's the right policy. If I look at the old serializer only the outer message was marked, the inner message is only an object of the outer.

namespace Microsoft.Testing.Platform.IPC.Models;

internal sealed record DiscoveredTestMessage(string? Uid, string? DisplayName, string? FilePath, int? LineNumber, string? Namespace, string? TypeName, string? MethodName, string[]? ParameterTypeFullNames, TestMetadataProperty[] Traits);
internal sealed record DiscoveredTestMessage(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we missing the attribute here?

namespace Microsoft.Testing.Platform.IPC.Models;

internal sealed record FileArtifactMessage(string? FullPath, string? DisplayName, string? Description, string? TestUid, string? TestDisplayName, string? SessionUid);
internal sealed record FileArtifactMessage(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we missing the attribute here?

namespace Microsoft.Testing.Platform.IPC.Models;

internal sealed record SuccessfulTestResultMessage(string? Uid, string? DisplayName, byte? State, long? Duration, string? Reason, string? StandardOutput, string? ErrorOutput, string? SessionUid);
internal sealed record SuccessfulTestResultMessage(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

[property: PipePropertyId(8)] string? SessionUid);

internal sealed record FailedTestResultMessage(string? Uid, string? DisplayName, byte? State, long? Duration, string? Reason, ExceptionMessage[]? Exceptions, string? StandardOutput, string? ErrorOutput, string? SessionUid);
internal sealed record FailedTestResultMessage(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

[property: PipePropertyId(4)] bool? IsBuiltIn);

internal sealed record CommandLineOptionMessages(string? ModulePath, CommandLineOptionMessage[]? CommandLineOptionMessageList) : IRequest;
[PipeSerializableMessage("DotNetTestProtocol", 3)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we extract the message ID constants to a static class?

</TfmSpecificPackageFile>
</ItemGroup>
<ItemGroup>
<Compile Remove="Q:\dev\testfx\src\Platform\Microsoft.Testing.Platform\IPC\Serializers\BaseSerializer.cs" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct, should be a relative path?

@nohwnd
Copy link
Member

nohwnd commented Nov 4, 2025

idea: Could the source generator output the source code into place where we check it in? I assume it will be developed further and it would make it easier to review the changes that happen between check-ins. The change in the protocol will have impact on compatibility of older versions of MTP with dotnet test (at least), I don't think we have any explicit lower bound there.

@Youssef1313
Copy link
Member

We also have some roundtrip tests in https://github.com/microsoft/testfx/blob/2001ece9cf87c9e5153b30d4b914d31941bdf976/test/UnitTests/Microsoft.Testing.Platform.UnitTests/IPC/ProtocolTests.cs

We could consider asserting the actual bytes so that we know when there is a change to them and then we will be able to carefully review its compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement a source generator for pipe protocol

4 participants