microsoft / testfx

MSTest framework and adapter
MIT License
714 stars 253 forks source link

TestContext.TestResultsDirectory is the same for all tests #502

Closed avivanoff closed 1 year ago

avivanoff commented 5 years ago

In MSTest V1 TestContext.TestResultsDirectory is different for every test. For instance:

TestMethod1 -> …\Deploy_1 2018-10-17 13_00_29\In\b3ce33a5-5c66-422e-960e-3cc97e6f7f13\MYMACHINE TestMethod2 -> …\Deploy_1 2018-10-17 13_00_29\In\7f4f9ba7-9081-479b-9f49-7d1e020604ff\MYMACHINE

After convertsion to MSTest V2:

TestMethod1 -> …\Deploy_1 2018-10-17 13_00_29\In TestMethod2 -> …\Deploy_1 2018-10-17 13_00_29\In

Is this a bug or should we update all our tests that rely on V1 behavior to account for the new one?

vagisha-nidhi commented 5 years ago

We have tried reproducing this issue, but weren't able to do so. We are getting only one TestContext.TestResultsDirectory for both MSTest V1 and MSTest V2 projects. Can you share sample repro projects for both MSTest V1 and V2 where you were able to get this behavior?

avivanoff commented 5 years ago

Add .testsettings file to MSTest V1 project.

avivanoff commented 5 years ago

This is what I found in DesktopTestContextImplementation.cs, line 171:

            // In MSTest, it is actually "In\697105f7-004f-42e8-bccf-eb024870d3e9\User1", but
            // we are setting it to "In" only because MSTest does not create this directory.
vagisha-nidhi commented 5 years ago

Hi @avivanoff I tried with the following code in MSTest v1 adding testsettings file to the project. I am still getting only one folder. The behavior seems to be consistent for both MSTest v1 and v2.

        [TestMethod]
        [DeploymentItem("a.testsettings")]
        public void TestMethod1()
        {
        }

        [TestMethod]
        [DeploymentItem("b.testsettings")]
        public void TestMethod2()
        {
        }

Is there anything else to be added to this? Can you send a repro project? Thanks.

avivanoff commented 5 years ago

Do not use .testsettings as a deployment item. Select them as Test Settings in Visual Studio when running tests, or as /Settings argument to vstest.console.exe.

vagisha-nidhi commented 5 years ago

I am still not able to repro this. I selected Test Settings file in Visual Studio when running tests. I am unable to get different folders in any case for MSTest V1. Can you please share a small repro project?

avivanoff commented 5 years ago

Sample project. UnitTest2.zip

parrainc commented 5 years ago

@vagisha-nidhi I was able to replicate this issue as described by @avivanoff. When running tests with Mstest v1, there's a different test run directory for each test (see example 1). On the other hand, when running with MsTest v2, there's just one test run directory for every test (see example 2).

Notice that running with both versions (v1 and v2) without the .testsettings file, it produces the same test result directory: ending in \In (no custom directory for each test).

example 1 (with MSTest V1):

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

        [TestMethod]
        public void TestMethod1() 
        {
            // with .testsettings: this outputs: "c:\users\{userid}\...\TestResults\user_MachineName 2018-10-25 06_59_55\In\505149a4-92ae-4469-a0a3-b03948fc9539\MachineName"
            // without .testsettings: outputs same as with MSTest V2
            this.TestContext.WriteLine(this.TestContext.TestResultsDirectory);
        }

        [TestMethod]
    public void TestMethod2()
    {
            // with .testsettings: this outputs: "c:\users\{userid}\...\TestResults\user_MachineName 2018-10-25 06_59_55\In\c35ef1a0-0388-4a15-b093-a94933a21250\MachineName"
            // without .testsettings: outputs same as with MSTest V2
        this.TestContext.WriteLine(this.TestContext.TestResultsDirectory);
    }
    }
}

example 2 (with MSTest V2):

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

        [TestMethod]
        public void TestMethod1() 
        {
            this.TestContext.WriteLine(this.TestContext.TestResultsDirectory); // this outputs: "c:\users\{userid}\...\TestResults\Deploy_user 2018-10-25 07_07_05\In"
        }

        [TestMethod]
    public void TestMethod2()
    {
       this.TestContext.WriteLine(this.TestContext.TestResultsDirectory); // this outputs: "c:\users\{userid}\...\TestResults\Deploy_user 2018-10-25 07_07_05\In"
    }
    }
}
vagisha-nidhi commented 5 years ago

Hi @avivanoff. I was able to get the repro here. We don't have .testsettings file supported witth MSTest v2 and don't plan to do so, hence there can be behavior difference when using testsettings file. The recommended way to do is to use .runsettings file Also, what is your scenario exactly for using these folder structures so that we can get you unblocked?

@parrainc MSTest v2 doesn't support testsettings so no tests are run in this case.

parrainc commented 5 years ago

@vagisha-nidhi Yup, i know. Just wanted to point out that in MsTest v1 without the .testsettings file it outputs the same results as in MsTest v2.

avivanoff commented 5 years ago

A lot of our tests rely on TestContext.TestResultsDirectory being unique to provide test isolation. And because most of our tests run in parallel, if TestContext.TestResultsDirectory is not unique, we have a lot of race conditions in terms of file access issues.

vagisha-nidhi commented 5 years ago

This is a valid ask but it's not a bug. The framework is designed in such a way to get only one TestContext.TestResultsDirectory for every test. For running tests in parallel, your tests should be designed such so that no resource race conditions occur while file accesses in these directories. Tagging @cltshivash and @PBoraMSFT to see if would like to take this up as enhancement.

avivanoff commented 5 years ago

The behavior between V1 and V2 changed. You should at least document it.

And thank you for all that extra work we now have to do.

jayaranigarg commented 5 years ago

@avivanoff : Apologies for the inconvenience but .testsettings are just not supported by MSTestV2. We have this documented here : https://github.com/Microsoft/testfx-docs/blob/master/docs/deltaWithMSTestV1.md

Tests should not even get discovered when you supply testsettings file with a MSTestv2 test project. How are you able to run your tests in parallel? Can you please attach a sample MSTestV2 based test project which you are using to hit this issue?

StingyJack commented 5 years ago

@avivanoff - If its less work, consider not upgrading those tests to the v2 adapter/framework. v1 is definitely more reliable and stable anyways,

one TestContext.TestResultsDirectory for every test

@vagisha-nidhi - I read that as "every test gets its own directory". Which sounds reasonable, as unit tests are supposed to be run in some kind of isolation (they arent integration tests). The adapter spins up app domains already, why would anyone take the chance of cross contaminating tests by coercing them to use the same filesystem resource when it can be easily avoided (and has been avoided in the past)?

The problem is that you mean "one directory for all tests to share", which sounds non-optimal considering it didnt do that before, and its probably not that difficult to fix compared to other work efforts.

For running tests in parallel, your tests should be designed such so that no resource race conditions occur while file accesses in these directories.

@jayaranigarg For this user the tests were designed so that no race conditions occurred. You (testfx team) dropped functionality and that created this condition, along with placing the burden of additional unplanned work on the users. Whether there was documentation or an announcement or not does not matter. We do not expect the tools we have purchased to make more work for us, ever. Tools are supposed to reduce the amount of effort required. The elegance, purity, or general shininess of a tool doesnt matter if the tool doesnt perform its basic purpose.

Evangelink commented 1 year ago

Given the number of upvote, it seems that this behavior is not so much of a pain for our users so I will move forward by closing the issue as won't fix.