microsoft / testfx

MSTest framework and adapter
MIT License
738 stars 255 forks source link

Async tests which use Assert.Inconclusive report as Error #249

Closed Terebi42 closed 7 years ago

Terebi42 commented 7 years ago

Description

Async tests which use Assert.Inconclusive report as Error

Steps to reproduce

   [TestMethod]
        public void TestInconclusiveSync()
        {

            Assert.Inconclusive("This will report as skipped");
        }

        [TestMethod]
        public async Task TestInconclusiveAsync()
        {

            Assert.Inconclusive("This will report as error");
        }

v1.1.18

AbhitejJohn commented 7 years ago

Thanks for filing this. I can repro this. I guess this seems to be happening because the error is thrown on another thread which gets wrapped in as an internal exception we do not check.

Terebi42 commented 7 years ago

This behavior is seen in several places, VS, TFS build, Resharper, etc. Do you think all of those would be resolved by this bug (because they are all relying on something in the nuget), or should I be filing bugs with those products? (because they will have to do this same exception peeking on their own)?

AbhitejJohn commented 7 years ago

@Terebi42 : Fixing this bug should take care of all the client scenarios.

abatishchev commented 7 years ago

I bugged the VSTS team with this just to learn down the road that it's unfortunately a bug in MSTest indeed. Any plans to address, and asap? It's 100% reproducible.

AbhitejJohn commented 7 years ago

@abatishchev: We have been busy with a few other things and this one is currently parked. I can try and help anyone from the community who is willing to take a stab at fixing this though. The code to invoke the test method is here

jessehouwing commented 7 years ago

I tried debugging this, which wasn't easy with all of the different test frameworks being loaded at the same time (sort of test framework inception). I think I may have found the issue where the Exception being thrown is from the MstestV2 test framework, but the exception being compared against is from the V1 framework. I am not a 100% sure, as my knowledge of the framework is limited.

I have forked and committed my changes here:

https://github.com/jessehouwing/testfx

AbhitejJohn commented 7 years ago

@Jessehouwing: Thanks. The change looks good to me. Can you please submit a PR? The multiple frameworks confusion that you were running into is limited to the Unit Test Framework in this repo only and isn't tied to the product code. It looks like ex is UTF.AssertInconclusiveException did the trick.

jthelin commented 7 years ago

I have the same issue, so I tried the "latest" myget daily build 1.2.0-build-20170919-01 which I think should contain the PR change #277 above, but i still have the async test case failing with error Assert.Inconclusive failed.

I used slightly different async test case code which included an await call, but it is still functionality equivalent to the example @Terebi42 gave originally.

[TestMethod]
public async Task Assert_Inconclusive_Async()
{
    await Task.Delay(1);

    Assert.Inconclusive("Assert_Inconclusive_Async");
}

Does the PR fix #277 work for async test cases for other people?

Or could a test case something like the above be added to the "official" test suite to confirm functionality?

jessehouwing commented 7 years ago

I'll add that locally. The test should be easy to add and check.

Just checked and it works:

[TestMethodV1]
        public void TestMethodInfoInvokeWaitAsyncShouldHandleThrowAssertInconclusive()
        {
            DummyTestClass.DummyAsyncTestMethodBody = () => Task.Run(async () =>
            {
                await Task.Delay(1);
                UTF.Assert.Inconclusive("Assert_Inconclusive_Async");
            });
            var asyncMethodInfo = typeof(DummyTestClass).GetMethod("DummyAsyncTestMethod");

            var method = new TestMethodInfo(
                asyncMethodInfo,
                this.testClassInfo,
                this.testMethodOptions);

            var result = method.Invoke(null);

            Assert.AreEqual(UTF.UnitTestOutcome.Inconclusive, result.Outcome);
        }
jthelin commented 7 years ago

Oops - my bad. I just noticed the "latest" build i was using from myget is 0919 [and not 0929 as i had thought] so it won't actually contain the #277 code fix.

So the real problem is that no "daily" builds have been produced in the last 10 days.

https://dotnet.myget.org/feed/mstestv2/package/nuget/MSTest.TestFramework

Could someone give the build hamsters a poke to wake them up?

jessehouwing commented 7 years ago

As far as I can tell your test case should succeed on the latest codebase.

jthelin commented 7 years ago

Thanks for confirming @jessehouwing

I should be able to use a local dev build for the moment, when I find a machine with UWP SDK installed.... ;)

abatishchev commented 7 years ago

Just in case my test method is the following:

[TestMethod]
public async Task TestAsync()
{
    await Task.Yield();

    Assert.Inconclusive();
}
AbhitejJohn commented 7 years ago

@jthelin : Sorry about that. The builds are failing because of service issues. Should be up this week hopefully.

jthelin commented 7 years ago

Confirming this issue is fixed for me now that v1.2.0 nuget packages published.

jayaranigarg commented 7 years ago

Thanks for confirming. Closing the issue.