Incorrect formatting of failure message if test fails with Assert.Multiple

[nvborisenko]

Assert.Multiple(() =>
            {
                Assert.AreEqual(2, 3, "The first failed assert");
                Assert.AreEqual(4, 5, "The second failed assert");
                throw new Exception("And exception and the end.");
            });

XML report contains failure message with missing whitespace:

[CharliePoole]

What is nunit3 console displaying for this test? We want the legacy failure message to match what nunit3 console displays as closely as possible.

[CharliePoole]

What I'm thinking is that the exception should be listed as the third item under the multiple failure list. But we may not have done it that way in the end.

[nvborisenko]

Console runner displays

7-1) Error : Example.Tests.Class1.TestMultipleAssertions
  The first failed assert
  Expected: 2
  But was:  3
   at Example.Tests.Class1.<>c.<TestMultipleAssertions>b__6_0() in C:\Projects\ReportPortal\example-net-nunit\src\Example\Tests\Class1.cs:line 79
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke) in D:\a\1\s\src\NUnitFramework\framework\Internal\AsyncToSyncAdapter.cs:line 50
   at NUnit.Framework.Assert.Multiple(AsyncTestDelegate testDelegate) in D:\a\1\s\src\NUnitFramework\framework\Assert.cs:line 353

7-2) Error : Example.Tests.Class1.TestMultipleAssertions
  The second failed assert
  Expected: 4
  But was:  5
   at Example.Tests.Class1.<>c.<TestMultipleAssertions>b__6_0() in C:\Projects\ReportPortal\example-net-nunit\src\Example\Tests\Class1.cs:line 80
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke) in D:\a\1\s\src\NUnitFramework\framework\Internal\AsyncToSyncAdapter.cs:line 50
   at NUnit.Framework.Assert.Multiple(AsyncTestDelegate testDelegate) in D:\a\1\s\src\NUnitFramework\framework\Assert.cs:line 353

7-3) Error : Example.Tests.Class1.TestMultipleAssertions
System.Exception : And exception and the end.
   at Example.Tests.Class1.<>c.<TestMultipleAssertions>b__6_0() in C:\Projects\ReportPortal\example-net-nunit\src\Example\Tests\Class1.cs:line 80
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke) in D:\a\1\s\src\NUnitFramework\framework\Internal\AsyncToSyncAdapter.cs:line 50
   at NUnit.Framework.Assert.Multiple(AsyncTestDelegate testDelegate) in D:\a\1\s\src\NUnitFramework\framework\Assert.cs:line 353

XML report

<failure>
              <message><![CDATA[System.Exception : And exception and the end.Multiple failures or warnings in test:
  1)   The first failed assert
  Expected: 2
  But was:  3

  2)   The second failed assert
  Expected: 4
  But was:  5

]]></message>
              <stack-trace><![CDATA[   at Example.Tests.Class1.<>c.<TestMultipleAssertions>b__6_0() in C:\Projects\ReportPortal\example-net-nunit\src\Example\Tests\Class1.cs:line 80
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke) in D:\a\1\s\src\NUnitFramework\framework\Internal\AsyncToSyncAdapter.cs:line 50
   at NUnit.Framework.Assert.Multiple(AsyncTestDelegate testDelegate) in D:\a\1\s\src\NUnitFramework\framework\Assert.cs:line 353]]></stack-trace>
            </failure>
            <assertions>
              <assertion result="Failed">
                <message><![CDATA[  The first failed assert
  Expected: 2
  But was:  3
]]></message>
                <stack-trace><![CDATA[   at Example.Tests.Class1.<>c.<TestMultipleAssertions>b__6_0() in C:\Projects\ReportPortal\example-net-nunit\src\Example\Tests\Class1.cs:line 79
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke) in D:\a\1\s\src\NUnitFramework\framework\Internal\AsyncToSyncAdapter.cs:line 50
   at NUnit.Framework.Assert.Multiple(AsyncTestDelegate testDelegate) in D:\a\1\s\src\NUnitFramework\framework\Assert.cs:line 353
]]></stack-trace>
              </assertion>
              <assertion result="Failed">
                <message><![CDATA[  The second failed assert
  Expected: 4
  But was:  5
]]></message>
                <stack-trace><![CDATA[   at Example.Tests.Class1.<>c.<TestMultipleAssertions>b__6_0() in C:\Projects\ReportPortal\example-net-nunit\src\Example\Tests\Class1.cs:line 80
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke) in D:\a\1\s\src\NUnitFramework\framework\Internal\AsyncToSyncAdapter.cs:line 50
   at NUnit.Framework.Assert.Multiple(AsyncTestDelegate testDelegate) in D:\a\1\s\src\NUnitFramework\framework\Assert.cs:line 353
]]></stack-trace>
              </assertion>
              <assertion result="Error">
                <message><![CDATA[System.Exception : And exception and the end.]]></message>
                <stack-trace><![CDATA[   at Example.Tests.Class1.<>c.<TestMultipleAssertions>b__6_0() in C:\Projects\ReportPortal\example-net-nunit\src\Example\Tests\Class1.cs:line 80
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke) in D:\a\1\s\src\NUnitFramework\framework\Internal\AsyncToSyncAdapter.cs:line 50
   at NUnit.Framework.Assert.Multiple(AsyncTestDelegate testDelegate) in D:\a\1\s\src\NUnitFramework\framework\Assert.cs:line 353]]></stack-trace>
              </assertion>
            </assertions>

Please advise what is preferred output.

[CharliePoole]

This will take some input from other @nunit/framework-team members.

I'll give some background, in case it helps...

When we introduced the assertion element in the XML, we expected runners to eventually take advantage of them to display output as they saw fit. In order not to break existing runners, we put a message incorporating potentially more than one assertion error into the original element.
I would hope that only older versions of runners are using this field at the present time, but I don't actually know.

@nvborisenko Did you encounter this message anywhere except in the XML output? IOW why is it important for you that this element be corrected. (Of course it should be in any case, but it's more important if some program is actually displaying it.)

The major difference between what the nunit3-console shows and what the console is able to display a separate stack trace for each failure. The legacy message field is just messages and there is only one stack trace, which is either that of the unexpected exception that terminated the block or the point at which the block was exited.

Also, in the legacy message, an exception that terminates the block is not listed as one of the items. I think we did it that way to avoid having separate paths for the case where there are already pending failures and the case where there are not.

We could fix this to make the message look more like what the console displays - listing the exception as one of the failure items. That's a more complicated fix than what @nvborisenko has done in his PR. Alternatively, we could go with the simpler fix, which I think really needs a couple of newlines rather than just a space. Thinking about it, I'm happy with the simpler fix.

Opinions?

[ChrisMaddock]

I've moved this issue over to the framework repo, where the fix will need to be. 🙂

I agree the simpler fix is fine. This is just the 'legacy' message - if someone wants to take the time to make the formatting equivalent, I'd happily merge the work - but it's not necessary in my eyes.