Skip to content

[Source Breaking (only for framework authors)]: Support test artifacts in VS #5323

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

Merged
merged 3 commits into from
Mar 28, 2025

Conversation

Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Mar 27, 2025

Fixes #4934

This change is a breaking change for framework authors. Framework authors will now need to add attachments to TestNode properties as TestFileArtifactProperty. There can be multiple of such property, or none at all.

Publishing TestNodeFileArtifact is no longer the way to report artifacts. The type is marked obsolete but is kept for binary compatibility.

Data consumers who used to consume TestNodeFileArtifact will instead need to consume TestNodeUpdateMessage and find the artifacts in the properties of the test node.

Tested in Playground:

code:

[TestClass]
public class TestClass
{
    public TestContext TestContext { get; set; }

    [TestMethod]
    [DataRow(0, UnfoldingStrategy = TestDataSourceUnfoldingStrategy.Fold)]
    [DataRow(1, UnfoldingStrategy = TestDataSourceUnfoldingStrategy.Fold)]
    public void TestMethod1(int x)
    {
        for (int i = 0; i < 5; i++)
        {
            TestContext.AddResultFile($"File #{i} for test case {x}");
        }
    }
}

image

@drognanar The order of attachments is reversed compared to VSTest, I think. But that seems to be on Test Explorer side.

FYI @bradwilson @thomhurst

Also @OsirisTerje. As you are relying on VSTestBridge, you will only need to bump MTP version so that attachments start to show correctly in Test Explorer.

@Youssef1313 Youssef1313 force-pushed the artifact branch 2 times, most recently from 91ef7b7 to 51da5fc Compare March 27, 2025 11:36
@microsoft-github-policy-service microsoft-github-policy-service bot added Area: MTP Belongs to the Microsoft.Testing.Platform core library Area: Server Mode labels Mar 27, 2025
Copy link
Contributor

@MarcoRossignoli MarcoRossignoli left a comment

Choose a reason for hiding this comment

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

Nit on name LGTM

@Youssef1313 Youssef1313 marked this pull request as ready for review March 27, 2025 13:37
@Youssef1313 Youssef1313 requested a review from nohwnd as a code owner March 27, 2025 13:37
@nohwnd
Copy link
Member

nohwnd commented Mar 27, 2025

Do you know who we are breaking with this? Is this all nunit, xunit, mstest and tunit?

@Youssef1313
Copy link
Member Author

Do you know who we are breaking with this? Is this all nunit, xunit, mstest and tunit?

xUnit and TUnit. For MSTest, we are always using the current version of VSTestBridge which accounts for the break in this PR. For NUnit, they are also good as they will just update VSTestBridge and that's it.

@nohwnd
Copy link
Member

nohwnd commented Mar 28, 2025

Does sound reasonable to me to make this change when we know the break is only for test framework authors. :) (how about Expecto?)

@Evangelink
Copy link
Member

Expecto also relies on the bridge so it's also only a matter of bumping version.

@thomhurst
Copy link
Contributor

Happy to make this change.

Session Artifacts will remain as they are today?

@Youssef1313
Copy link
Member Author

@thomhurst Yes, session artifacts are still the same. This only affects TestNodeFileArtifact which previously didn't show up in VS at all.

@Youssef1313 Youssef1313 changed the title [Breaking]: Support test artifacts in VS [Source Breaking (only for framework authors)]: Support test artifacts in VS Mar 28, 2025
@Youssef1313 Youssef1313 merged commit 5404d3f into microsoft:main Mar 28, 2025
8 checks passed
@Youssef1313 Youssef1313 deleted the artifact branch March 28, 2025 13:11
stan-sz pushed a commit to stan-sz/testfx that referenced this pull request Apr 1, 2025
@bradwilson
Copy link

Is this change pushed to NuGet yet? If not, is there a planned release vehicle & timing?

@Youssef1313
Copy link
Member Author

@bradwilson It will be available in 1.7. We may want to release a preview of 1.7 sometime soon.

@bradwilson
Copy link

Available in xUnit.net v3 3.0.0-pre.4 https://xunit.net/docs/using-ci-builds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: MTP Belongs to the Microsoft.Testing.Platform core library Area: Server Mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is TestNodeFileArtifact the way we report attachments in MTP?
6 participants