Skip to content
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

Add task aware string writer with asynclocal #940

Merged
merged 19 commits into from
Nov 23, 2021

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Aug 18, 2021

Change implementation of ThreadSafeStringWriter to use AsyncLocal static that will avoid stealing the "context" when another ThreadSafeStringWriter is created. Add parameter to distinguish the different kinds of output that we capture because otherwise all output types would all go into the same string writer (e.g., Debug and Console output for one test would be put together).

This implementation is not great, but it avoids putting the implementation into the .Core which is Portable and can't have AsyncLocal.

It works nicely with non-async parallel methods as well as with async parallel methods, and works with non-parallel execution as well.

The setup and cleanup output is still a bit problematic, because setup runs in the context of one of the tests, and we don't know which test will end up capturing it. The cleanup output is not shown in TE but is shown in console because it is sent out as a message (not my change, this is how it always worked in v2).

It would be also nice to surface All output (currently only capturing this in Debug build), to see in which order the output was produced, which might be useful in some cases, but would get overwhelming fast.

  • implement for net core
  • share implementation with .net framework - we build against net45 which does not support asynclocal, net46 is the minimal version.
  • change the identifiers of the different outputs?
  • keep the old implementation for ns10 working

Further improvements:

  • associate teardown output to something, or at least add the type name
  • have all output in chronological order associated with the run and shown in TE somewhere

@nohwnd
Copy link
Member Author

nohwnd commented Aug 25, 2021

Test proj:

<!-- file TestProject29.csproj -->
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net5.0</TargetFramework>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.9.4" />
    <PackageReference Include="MSTest.TestAdapter" Version="99.99.99-dev" />
    <PackageReference Include="MSTest.TestFramework" Version="99.99.99-dev" />
    <PackageReference Include="coverlet.collector" Version="3.0.2" />
  </ItemGroup>

</Project>
// file UnitTest1.cs
using Microsoft.VisualStudio.TestTools.UnitTesting;
using System;
using System.Threading.Tasks;

[assembly: Parallelize(Workers = 0, Scope = ExecutionScope.MethodLevel)]

namespace TestProject29
{
    [TestClass]
    public class UnitTest1
    {
        public TestContext TestContext { get; set; }


        [ClassInitialize]
        public static async Task SetupTests(TestContext testContext)
        {
            testContext.WriteLine("test context in setup");
            Console.WriteLine("class setup");
            System.Threading.Thread.Sleep(1);
        }

        [ClassCleanup]
        public static async Task CleanupTests()
        {
            Console.WriteLine("class cleanup");
        }

        public static bool Flag2 { get; private set; }
        public static bool Flag3 { get; private set; }

        [TestMethod]
        [DataRow(1)]
        [DataRow(2)]
        public async Task TestMethod1(int a)
        {
            TestContext.WriteLine("test context in test");
            Console.WriteLine($"test1-{a}");
            Console.WriteLine($"test1-{a}");
            Console.WriteLine($"test1-{a}");
            while (!UnitTest1.Flag2)
            {
            }
            Console.WriteLine($"test1-{a}");
            Console.WriteLine($"test1-{a}");
            Console.WriteLine($"test1-{a}");
            while (!UnitTest1.Flag3)
            {
            }
            Console.WriteLine($"test1-{a}");
            Console.WriteLine($"test1-{a}");
            Console.WriteLine($"test1-{a}");
        }

        [TestMethod]
        public async Task TestMethod2()
        {
            Console.WriteLine("test2");
            Console.WriteLine("test2");

            UnitTest1.Flag2 = true;

            Console.WriteLine("test2");
            Console.WriteLine("test2");
        }

        [TestMethod]
        public async Task TestMethod3()
        {
            Console.WriteLine("test3");
            Console.WriteLine("test3");

            UnitTest1.Flag3 = true;

            Console.WriteLine("test3");
            Console.WriteLine("test3");
        }

    }

    [TestClass]
    public class UnitTest4
    {
        [TestMethod]
        public async Task TestMethod4()
        {
            Console.WriteLine("test4");
            Console.WriteLine("test4");
            Console.WriteLine("test4");
            throw new Exception("aaa");
        }
    }
}

Current version:

image
image

2.1.0:

image

@nohwnd
Copy link
Member Author

nohwnd commented Oct 8, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nohwnd nohwnd marked this pull request as ready for review November 22, 2021 14:45
@nohwnd
Copy link
Member Author

nohwnd commented Nov 22, 2021

TestProject42.zip
Here is a test proj if you want to take it for spin.

Copy link
Contributor

@Haplois Haplois left a comment

Choose a reason for hiding this comment

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

LGTM, verifying signing - we can merge after that.

@Haplois Haplois enabled auto-merge (squash) November 23, 2021 18:13
@Haplois Haplois merged commit 3904a64 into microsoft:main Nov 23, 2021
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.

2 participants