spekt / junit.testlogger

JUnit test logger for vstest platform (<= v3.x)
MIT License
77 stars 15 forks source link

Option to publish test name as published by framework #57

Open tamaramironov opened 2 years ago

tamaramironov commented 2 years ago

MethodFormat option to publish test name as published by framework

This used to be a default option, however it was removed while resolving another issue. Described in detail here

GMillerVarian commented 1 year ago

Any update on this? We are facing the same issue, and are therefore stuck on version 3.0.98 until this is resolved.

Siphonophora commented 1 year ago

@GMillerVarian Can you give an example of what testing framework you are using and how you are setting the name of the method?

Siphonophora commented 1 year ago

By the way, I spot checked @tamaratomilova example from #30, and it does look like the RMS name should be getting logged.

using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestMethod("@RMS_TEST_NUMBER RMS Test name")]
public async Task MethodUnderTest_Scenario_Result()
{
    Assert.Whatever();
}
GMillerVarian commented 1 year ago

I was able to repro with the following:

using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestMethod("@RMS-TEST-1 RMS Test without data rows")]
public void ValidateTest1()
{
    Validate(_sut, WritePolicy, "DeleteAsync");
}

[TestMethod("@RMS-TEST-2 RMS Test with data rows")]
[DataRow(ReadPolicy, "GetAsync")]
[DataRow(WritePolicy, "DeleteAsync")]
public void ValidateTest2(string policyName, string methodName)
{
    Validate(_sut, policyName, methodName);
}

Before (JunitXml.TestLogger 3.0.98):

<testcase classname="ValidateTests" name="@RMS-TEST-1 RMS Test without data rows" time="0.0005247" />
<testcase classname="ValidateTests" name="@RMS-TEST-2 RMS Test with data rows" time="0.0004483" />
<testcase classname="ValidateTests" name="@RMS-TEST-2 RMS Test with data rows" time="0.0003163" />

After (JunitXml.TestLogger 3.0.124):

<testcase classname="ValidateTests" name="@RMS-TEST-1 RMS Test without data rows" time="0.0004905" />
<testcase classname="ValidateTests" name="ValidateTest2 (ReadPolicy,GetAsync)" time="0.0004159" />
<testcase classname="ValidateTests" name="ValidateTest2 (WritePolicy,DeleteAsync)" time="0.0002794" />

So test 1 is fine, but we lose traceability in our RMS for test 2 with the DataRows.

Siphonophora commented 1 year ago

@GMillerVarian I'm having trouble re-producing that.


* UnitTest1.cs
``` cs
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace NUnit.Xml.TestLogger.NetFull.Tests
{
    [TestClass]
    public class UnitTest1
    {
        [TestMethod("@RMS_TEST_NUMBER RMS Test name")]
        public void MethodUnderTest_Scenario_Result()
        {
            Assert.Fail();
        }

        [TestMethod("@RMS-TEST-2 RMS Test with data rows")]
        [DataRow("GetAsync")]
        [DataRow("DeleteAsync")]
        public void ValidateTest2(string methodName)
        {
            Assert.Fail();
        }
    }
}

dotnet test --logger:junit Gives the same result as your 'after' example. Can you give me a minimal example like this?

GMillerVarian commented 1 year ago

It looks like there is some inconsistent behavior depending on whether tests include rows with param of "Type" or "Type[]". Try adding the following tests (apologies, this is like what I had originally tested with, but I had edited the results to simplify things - not realizing this was actually the cause):

    [TestMethod("@RMS-TEST-3 RMS Test 3")]
    [DataRow("PostAsync", null)]
    [DataRow("GetAsync", typeof(string))]
    [DataRow("DeleteAsync", typeof(int))]
    public void ValidateTest3(string methodName, Type type)
    {
        Assert.Fail($"Failed on {methodName}");
    }

    [TestMethod("@RMS-TEST-4 RMS Test 4")]
    [DataRow("PostAsync")]
    [DataRow("GetAsync", typeof(string), typeof(string))]
    [DataRow("DeleteAsync", typeof(int))]
    public void ValidateTest4(string methodName, params Type[] extraParams)
    {
        Assert.Fail($"Failed on {methodName}");
    }

We can also see this with xUnit:

    [Fact(DisplayName = "@RMS-Test-1 Test 1")]
    public void Test1()
    {
        Assert.False(false);
    }

    [Theory(DisplayName = "@RMS-Test-2 Test 2")]
    [InlineData("param1")]
    [InlineData("param2")]
    public void Test2(string param)
    {
        Assert.False(false);
    }

Before (JunitXml.TestLogger 3.0.98):

    <testcase classname="TestProject1.UnitTest1" name="@RMS-Test-2 Test 2(param: &quot;param2&quot;)" time="0.0027804" />
    <testcase classname="TestProject1.UnitTest1" name="@RMS-Test-2 Test 2(param: &quot;param1&quot;)" time="0.0000143" />
    <testcase classname="TestProject1.UnitTest1" name="@RMS-Test-1 Test 1" time="0.0000507" />

After (JunitXml.TestLogger 3.0.124):

    <testcase classname="TestProject1.UnitTest1" name="Test2(param: &quot;param2&quot;)" time="0.0020273" />
    <testcase classname="TestProject1.UnitTest1" name="Test2(param: &quot;param1&quot;)" time="0.0000086" />
    <testcase classname="TestProject1.UnitTest1" name="Test1" time="0.0000483" />
Siphonophora commented 1 year ago

That helped a bit.

So, test explorer and the loggers are seeing 4 tests (numbered in the image below) with 8 total results when we test your updated MSTest example. Each of those 4 tests can be run individually in test explorer. The tests using the display names each have 3 results, and the ones using the actual method names just have one.

In the 'best case' output using junit 3.0.98, we get 6 results using the display name and 2 using the method name. @GMillerVarian is that what you would expect or would want an option to output?

image

2023-04-27_18_16_47.trx.txt TestResults junit 3.0.98.xml.txt TestResults junit 3.0.124.xml.txt

@codito I'm surprised by the existence of the tests I numbered 2 and 4 above. But they seem to be real, and not linked to the others when they get logged. I'm including trx logger here as a cross check, and the GUIDs agree with that assessment. That suggest to me that any implementation which logs the display name is going to seem like logging the wrong thing in many places (see the 3.0.98 log). Also seems like we would need a feature flag for this on the core logger. What do you think?

GMillerVarian commented 1 year ago

We expect to see the 6 tests cases shown using display name (i.e. including the @RMS tag). This is what I see if I port that example to xUnit using junit 3.0.98. I'm not sure about the other 2 results with the method name, they seem redundant - but since they don't include the @RMS tag they should be irrelevant when we publish test results to our RMS.

Whether this is the default behavior or requires us to specify an option doesn't matter, as long as there is some way for us to get this behavior.

GMillerVarian commented 1 year ago

I would also expect to see the same 6 test cases in Test Explorer, which is what we see with xUnit:

image

Although this matters little to us, our only real concern here is the RMS tags for traceability.

GMillerVarian commented 1 year ago

@codito @Siphonophora Any update on this issue?

codito commented 1 year ago

@GMillerVarian @Siphonophora I think we have multiple issues here. (1) below is in scope for this logger. (2) and (3) are test framework specific concerns, we can take those to https://github.com/Microsoft/testfx repo.

(1) Behavior change between junit 3.0.98 and junit 3.0.124

That suggest to me that any implementation which logs the display name is going to seem like logging the wrong thing in many places (see the 3.0.98 log).

I agree with this. We should make MethodFormat tokenizable similar in spirit to LogFileName. E.g., dotnet test --logger:junit;MethodFormat={displayname} {method}, the possible tokens can be sourced from TestResultInfo data structure.

So far, the decision of what to report in MethodFormat is only configurable for junit. I'd suggest to keep it that way for now. If there's enough ask, we can push this functionality to core TestLogger like the LogFileName feature. @Siphonophora what do you think?

(2) MSTest reporting 8 test results vs xUnit reporting 6 results

2 extra test results in MSTest appears like a bug to me. Re https://github.com/spekt/junit.testlogger/issues/57#issuecomment-1526665206, trx also has 8 tests. Although trx is unique in supporting the parent/child relationship: we have two tests (starting with RMS tag as specified in TestMethod ctor) with 3 test results.

(3) MSTest using different displayName than xUnit

Hard to say whether this is a bug or by design. Test Explorer supports the same hierarchical model like trx.

codito commented 1 year ago

@Siphonophora we're now exposing DisplayName in TestResultInfo. It uses TestCaseDisplayName as default, but is available for us to add tokenization support as proposed in above comment.

GMillerVarian commented 10 months ago

@codito @Siphonophora Are you able to provide an update as to when we can expect this to be supported?

codito commented 10 months ago

@GMillerVarian sorry, we don't have an ETA for this.

If someone from the community would like to create a PR, I will be happy to support with review, merge and cutting a release. /cc:@Siphonophora