Change Apex tests from Xunit to MSTest #4306

heng-liu · 2021-10-14T00:17:30Z

Bug

Fixes: https://github.com/nuget/client.engineering/issues/852

Regression? Last working version:

Description

Background
Since Xunit is not able to timeout individual test (comment), in order to time out individual Apex tests, change Apex tests from Xunit to MSTest.

1.Change Apex test from Xunit to MSTest:

Change Xunit attributes to MSTest attributes: Theory -> TestMethod, InlineData -> DataRow, Skip -> Ignore, etc.
Add Timeout attribute for each Apex tests
Change Xunit.Assert to MSTest assertion
Adapt setup of Xunit (fixture) to MSTest setup (ClassInitialize). (MSTest has limitation as ClassInitialize is not inheritable. So change the previous fixture for base test class to a static property.)
Change Xunit log into Trace.WriteLine. (a test build shows that when the test failed, the log will be shown)

Engineering change

Add package for MSTest: MSTest.TestAdapter and MSTest.TestFramework
Change build/template.runsettingsproj to add console logger, so that trace will be shown in the log.
Clean up test/NuGet.Tests.Apex/NuGet.Tests.Apex/xunit.runner.json
Decide not to clean up xunit packages from Apex test projects, as Apex test projects reference Test.Utility project, which has xunit package reference. We will have to get rid of all references to xunit from Test.Utility project first. I don't think it's worth doing compared to the effort (The only impact is we still see several xunit message in test logs).

PR Checklist

This reverts commit d3400ff.

erdembayar

Start with few comments. In general looks good.

erdembayar · 2021-10-14T15:14:18Z

build/packages.targets

@@ -113,6 +113,8 @@
        <PackageReference Update="xunit" Version="$(XunitVersion)" />
        <PackageReference Update="xunit.runner.visualstudio" Version="$(XunitVersion)" />
        <PackageReference Update="Xunit.StaFact" Version="0.2.9" />
+        <PackageReference Update="MSTest.TestAdapter" Version="2.1.1" />
+        <PackageReference Update="MSTest.TestFramework" Version="2.1.1" />


Why this version 2.2.1 is chosen? It looks latest version is 2.2.7.

erdembayar · 2021-10-14T15:21:25Z

test/NuGet.Tests.Apex/NuGet.Tests.Apex/NuGetEndToEndTests/NetCoreProjectTestCase.cs

    {
-        public NetCoreProjectTestCase(VisualStudioHostFixtureFactory visualStudioHostFixtureFactory, ITestOutputHelper output)
-            : base(visualStudioHostFixtureFactory, output)
+        private const int Timeout = 5 * 60 * 1000; // 5 minutes


Maybe move this SharedVisualStudioHostTestClass.cs it looks repeated.

erdembayar · 2021-10-14T15:27:19Z

test/NuGet.Tests.Apex/NuGet.Tests.Apex/NuGetEndToEndTests/NetCoreProjectTestCase.cs

-        [NuGetWpfTheory]
-        [MemberData(nameof(GetNetCoreTemplates))]
-        public void CreateNetCoreProject_RestoresNewProject(ProjectTemplate projectTemplate)
+        [TestMethod]


Do we need to change NuGetWpfTheory to MSTest attribute or created new one?

NuGet.Client/test/TestExtensions/NuGet.StaFact/NuGetWpfTheoryAttribute.cs

Line 10 in 8923757

[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]

kartheekp-ms · 2021-10-20T22:08:20Z

If I am reading xunit/xunit#217 (comment) from the xUnit maintainer correctly, the framework supports timeout parameter for async tests. May we should change one async test locally and check if timeout works.

erdembayar · 2021-10-20T22:15:39Z

test/NuGet.Tests.Apex/NuGet.Tests.Apex/Apex/ApexTestContext.cs

        {
-            logger.LogInformation("Creating test context");
+            Trace.WriteLine("Creating test context");


How we're monitoring this message? If I remember correctly then it writes to VS "Output window". How do we check this?

Sorry for the late response..
I think logger.LogInformation is not writing to VS "Output window", but print log to the testing console.
If checking the following code:
1.ApexTestContext accept ILogger logger in the constructor at here
2.One example of new a ApexTestContext object is this
3.And the XunitLogger is from here
4.If checking the constructor of XunitLogger, it's here
We need the ITestOutputHelper as in Xunit, there seems no other way to print log in the testing console.
Pls check https://xunit.net/docs/capturing-output

For MSTest, we could use Trace.WriteLine to log, the output is written to an instance of DefaultTraceListener.
And in the testing, we could see the trace is shown when test failed in this build

kartheekp-ms · 2021-10-20T23:54:00Z

If I am reading xunit/xunit#217 (comment) from the xUnit maintainer correctly, the framework supports timeout parameter for async tests. May we should change one async test locally and check if timeout works.

I just running an apex test locally with a timeout and disabling parallel execution of tests. Unfortunately, tests didn't stop after timeout.

heng-liu · 2021-10-20T23:54:25Z

If I am reading xunit/xunit#217 (comment) from the xUnit maintainer correctly, the framework supports timeout parameter for async tests. May we should change one async test locally and check if timeout works.

Thanks @kartheekp-ms ! I checked the issue, too. But there are two Apex test classes IVsServicesTestCase and NetCoreProjectTestCase, having not async test cases, which will still not be able to timed out if using Xunit.

ghost · 2021-11-05T22:00:42Z

This PR has been automatically marked as stale because it has no activity for 7 days. It will be closed if no further activity occurs within another 7 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

heng-liu · 2023-03-10T16:59:02Z

Found a related tickets I raised at that time: https://dev.azure.com/mseng/AzureDevOps/_workitems/edit/1885534

heng-liu added 5 commits October 13, 2021 11:52

start working..

4385865

make tests to fail, see if trace is printed

d3400ff

clean up

05af89a

Revert "make tests to fail, see if trace is printed"

34035f2

This reverts commit d3400ff.

add [TestMethod] for a ignored test

4b2e52c

heng-liu marked this pull request as ready for review October 14, 2021 03:48

heng-liu requested a review from a team as a code owner October 14, 2021 03:48

erdembayar reviewed Oct 14, 2021

View reviewed changes

erdembayar reviewed Oct 20, 2021

View reviewed changes

ghost added the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Nov 5, 2021

ghost closed this Nov 13, 2021

nkolev92 deleted the dev-hengliu-CI-chanageApex-mstest2 branch April 8, 2022 18:54

heng-liu restored the dev-hengliu-CI-chanageApex-mstest2 branch January 25, 2023 07:08

nkolev92 deleted the dev-hengliu-CI-chanageApex-mstest2 branch February 23, 2023 00:08

ghost removed the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Mar 10, 2023

heng-liu restored the dev-hengliu-CI-chanageApex-mstest2 branch July 6, 2023 16:59

nkolev92 deleted the dev-hengliu-CI-chanageApex-mstest2 branch July 2, 2024 20:41

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change Apex tests from Xunit to MSTest #4306

Change Apex tests from Xunit to MSTest #4306

heng-liu commented Oct 14, 2021 •

edited

Loading

erdembayar left a comment

erdembayar Oct 14, 2021

erdembayar Oct 14, 2021

erdembayar Oct 14, 2021

kartheekp-ms commented Oct 20, 2021

erdembayar Oct 20, 2021

heng-liu Oct 29, 2021

kartheekp-ms commented Oct 20, 2021 •

edited

Loading

heng-liu commented Oct 20, 2021

ghost commented Nov 5, 2021

heng-liu commented Mar 10, 2023

Change Apex tests from Xunit to MSTest #4306

Change Apex tests from Xunit to MSTest #4306

Conversation

heng-liu commented Oct 14, 2021 • edited Loading

Bug

Description

PR Checklist

erdembayar left a comment

Choose a reason for hiding this comment

erdembayar Oct 14, 2021

Choose a reason for hiding this comment

erdembayar Oct 14, 2021

Choose a reason for hiding this comment

erdembayar Oct 14, 2021

Choose a reason for hiding this comment

kartheekp-ms commented Oct 20, 2021

erdembayar Oct 20, 2021

Choose a reason for hiding this comment

heng-liu Oct 29, 2021

Choose a reason for hiding this comment

kartheekp-ms commented Oct 20, 2021 • edited Loading

heng-liu commented Oct 20, 2021

ghost commented Nov 5, 2021

heng-liu commented Mar 10, 2023

heng-liu commented Oct 14, 2021 •

edited

Loading

kartheekp-ms commented Oct 20, 2021 •

edited

Loading