Provide multiple results for a test case in the XML output

[CharliePoole]

This feature is needed in order to support

• Multiple Asserts - so that multiple failures can be reported
• Warning-level Asserts - so that both warnings and any eventual failure can be reported
• General assertion reporting - so that all asserts can be reported

Since MultipleAsserts is queued up first, I'm making it a part of that Epic.

[CharliePoole]

Initially, we will have to redesign the xml result format to support multiple assertion results per test.
Currently, each test can only have a single result, which is the result of the final assertion or a default success result if no assertions failed.

The final assertion may also be an Assert.Pass, Assert.Fail, Assert.Inconclusive or Assert.Ignore, each of which always terminates the test. Any assertions prior to that final assertion is necessarily successful.

When we implement multiple Assertions (#391) a test may have more than one failing assert result. We need to decide how to reflect this in the object model and the XML result. An "obvious" approach would be to create a <results> or <asserts> element containing one or more individual <result> or <assert> elements. However, this could break existing runners as well as user code that processes the XML result file.

One approach for backward compatibility would be to support both the existing elements and the new elements for a period of time. They could co-exist in the same output or be selected based on a setting provided by the runner and defaulting to the older format. More sophisticated runners - including all the NUnit runners - would use the newer format while legacy runners would continue to see what they are used to seeing.

Whatever we select will require implementation across the framework, engine and each runner, so it's a fairly big deal. Comments please?

[remcomulder]

I see broadly 3 ways of going about introducing this change:

1. Replace the existing implementation with a new XML element containing multiple results (not recommended as this won't have any backwards compatibility with older runners)
2. Introduce a new XML element inside the existing structure that contains multiple results. The first result is still always returned in the previous manner, so older runners can still work with it. I think it's reasonable to expect that older runners will likely ignore elements they don't understand, so this gives a good chance of backwards compatibility. The downside is that it makes the XML appear much more complex and harder to understand, increasing the risk of problems for people newly integrating with the API.
3. Add a config switch to change the output entirely to the new format. In 2-3 years time as people progressively update their runners, it could be possible to switch over entirely to the new format. Drawback is increased complexity in the NUnit codebase.

My preference is option 3. Option 2 might work for me (I'd need to double-check to be sure I'm not doing anything crazy here).

Probably also be good to have the opinions of @jcansdale and @citizenmatt

[CharliePoole]

That matches my view of the options as well with the addition of a choice under option 3:

3.1 Switch would turn on the new multiple result output. Default would be old format.

3.2 Switch would turn off the old elements. Default would be mixed.

In all cases, the attributes of each test case would remain unchanged. That is, the result attribute would continue to reflect overall success and failure.

Separate from the format issue is the question of whether the runner setting disables the framework feature. I would say no, but there are arguments both ways.

[jcansdale]

Another approach would be to add support for System.AggregateException. This would allow a test to either pass, or fail for one or more reasons. Here's a post about System.AggregateException:
https://msdn.microsoft.com/en-us/magazine/ee321571.aspx

The multi-failure could be represented in a similar way to the AggregateException object itself. At the top level all information would be available in the existing message and stack-trace elements. Nested exception objects could be represented as nested failure elements (like @remcomulder's 2).

Something like this:

            <failure>
              <message>
                Assert message 1
                Assert message 2
              </message>
              <stack-trace>...</stack-trace>
              <failure>
                <message>Assert message 1</message>
                <stack-trace>...</stack-trace>
              </failure>
              <failure>
                <message>Assert message 2</message>
                <stack-trace>...</stack-trace>
              </failure>
            </failure>

As well as allowing support for multi-asserts, this might also cater for scenarios involving async/tasks where multiple tasks might throw.

I'll elaborate more later, but I just wanted to throw it out there. 😄

[remcomulder]

I really like the idea of using System.AggregateException. This would slot perfectly into the existing handling for test exceptions, and it would give runner authors a way to extend test result handling in a more standardised way.

It also means that if people wanted to report multiple exceptions outside the NUnit assertion system, it would be much easier for them to do that. Many people these days are now using NUnit along side other assertion libraries (i.e. Shouldly etc).

The runner implementation would simply dissect the AggregateException and pull out its contents, then hold multiple exceptions against the test.

[oznetmaster]

System.AggregateException is not available prior to .NET 4.0.

If it were to be used, then it would have to be added to the compatibility classes for NET 2.0, NET 3.5, and NETCF.

[jcansdale]

I hadn't thought about support for 3rd party assertion libraries. That's probably a much bigger deal than support for async/tasks that might throw multiple exceptions!

I've prototyped a couple of implementations, one that takes multiple Actions and one that takes a single Action (like the MbUnit example).

They already work with existing runners, but the stack trace could do with a little filtering. Here is how it looks with NCrunch (the side markers) and MSTest (the top markers):

As you can see the multiple Actions implementation works better with NCrunch (you can see exactly which assertions are failing).

Here are a few pros and cons with the two implementations.

Multiple Action (AssertMultipleX above)

• Simple implementation
• Doesn't need cooperation of Assert methods.
• Works with 3rd party assert libraries
• Failing assert is visible to test runners that highlight thrown exceptions.

    public class Assert
    {
        public static void Multiple(params Action[] asserts)
        {
            var exceptions = new List<Exception>();
            foreach (var assert in asserts)
            {
                try
                {
                    assert();
                }
                catch (Exception e)
                {
                    exceptions.Add(e);
                }
            }

            if (exceptions.Count > 0)
            {
                throw new AggregateException(exceptions);
            }
        }
    }

Single Action (AssertMultipleY above)

• Syntax is perhaps a little cleaner?
• Requires cooperation of Assert methods.
• Wouldn't work with 3rd party assert libraries that throw
• Exposing line numbers would require error prone use of new StackFrame(skipFrames, true)

    public class Assert
    {
        [ThreadStatic]
        static List<Exception> exceptions;

        [ThreadStatic]
        static bool multiple;

        public static void Multiple(Action action)
        {
            try
            {
                multiple = true;
                action();
                if (exceptions != null)
                {
                    throw new AggregateException(exceptions);
                }
            }
            finally
            {
                exceptions = null;
                multiple = false;
            }
        }

        // repeat for all other assert methods
        public static void That<TValue>(TValue actual, IResolveConstraint constraint)
        {
            try
            {
                NUnit.Framework.Assert.That(actual, constraint);
            }
            catch (Exception e)
            {
                if(!multiple)
                {
                    throw;
                }

                if (exceptions == null)
                {
                    exceptions = new List<Exception>();
                }

                exceptions.Add(e);
            }
        }
    }

You can try the above prototypes my dropping them in as a nested class.

[CharliePoole]

Catching up on comments... I got behind while working on other stuff.

@jcansdale
I think whether we use AggregateException or not isn't too relevant to this issue, since the exception never escapes the framework. I created this separate issue to deal with what info the framework will provide to the outside world, because otherwise we seem to keep coming back to syntax, which already has several good options udner discussion elsewhere.

Your proposed expansion of the <failure> element works for failures but not for reporting warnings or successes. My inclination would be to phase out <failure> in favor of something like...

            <asserts>
              <assert result="failed">
                <message>Assert message 1</message>
                <stack-trace>...</stack-trace>
              </assert result="failed">
              <assert result="failed">
                <message>Assert message 2</message>
                <stack-trace>...</stack-trace>
              </assert>
            </asserts>

We could keep <failure> around for a while for backward compatibility, perhaps concatentating messages as you suggest. We might do the same for the` element.

[jcansdale]

@CharliePoole,

I think whether we use AggregateException or not isn't too relevant to this issue, since the exception never escapes the framework.

As @remcomulder picks up on, there is an already common scenario where AggregateException or at least non-NUnit specific exceptions are thrown from outside the framework. I'm quoting him here:

I really like the idea of using System.AggregateException. This would slot perfectly into the existing handling for test exceptions, and it would give runner authors a way to extend test result handling in a more standardised way.

It also means that if people wanted to report multiple exceptions outside the NUnit assertion system, it would be much easier for them to do that. Many people these days are now using NUnit along side other assertion libraries (i.e. Shouldly etc).

The runner implementation would simply dissect the AggregateException and pull out its contents, then hold multiple exceptions against the test.

From the POV of integrators who need to support multiple unit testing frameworks, having a simple and standard way of reporting multiple assertions (from potentially mixed assertion libraries) would be very welcome!

The fact that it can be implemented in a few lines of code is surely a good thing. 😄

Simple Assert.Multiple implementation

    public class Assert
    {
        public static void Multiple(params Action[] asserts)
        {
            var exceptions = new List<Exception>();
            foreach (var assert in asserts)
            {
                try
                {
                    assert();
                }
                catch (Exception e)
                {
                    exceptions.Add(e);
                }
            }

            if (exceptions.Count > 0)
            {
                throw new AggregateException(exceptions);
            }
        }
    }


[oznetmaster]

Unless it is added as a compatibility file, AggregateException cannot be used on any pre 4.0 build

[CharliePoole]

I agree with you 100% that we ought to support AggregateException, for many reasons. In fact , we don't currently support it and the example you give won't be handled correctly by NUnit. Your exception will be treated as an error in both the framework and engine, not as a test failure. The individual asserts will not be reported separately unless we add additional code upstream to do it, so it's less simple than you state. In fact, there is also a simple implementation of the requested syntax in a "few lines of code." But, like your example, it's not the complete implementation. As you know, prototypes are usually easy. It's the last bits that take most of the work.

[CharliePoole]

@oznetmaster Just to be clear... I don't think we should use AggregateException, jut that we should support it when it occurs in calls to external code. However, contrary to @jcansdale, I don't think it has anything to do with this issue.

@jcansdale You were the primary reason I created an Epic with multiple issues. It's hard to have a discussion if we don't all focus on the same thing. This issue is about the form of the result file that will be produced when a test produces results from multiple asserts, some of which may have failed, some passed, some issued (future) warnings.

I think discussions of alternate syntax are important too. I see that as completely orthogonal to what this issue is about. @yetibrain just suggested a different alternative and you already have a separate issue (#1737) that discusses the same alternative you are pushing here. Can we please keep the discussion of syntax separate from the discussion of multiple test results?

[CharliePoole]

I should also add... the reason I'm focused on getting the form of the output right is two-fold:

1. I think that what we want to produce should drive everything else.
2. The syntactic part is more or less trivial to implement for all alternatives, while the result file format is complex because of the many uses to which it is put.

I just changed the title of this issue, in case the existing one was confusing things. Let's keep the discussion here on topic, please.

Should we create an issue under the Epic that deals with the proposed alternative syntactic approaches? My feeling was that everyone involved had agreed on a particular syntax about a year ago, but we can re-open it if folks don't like what was selected from a user point of view. I'm not particularly interested in evaluating the syntax on the basis of ease of implementation unless some option looks particularly burdensome to us.

[jcansdale]

@CharliePoole,

Should we create an issue under the Epic that deals with the proposed alternative syntactic approaches?

I think that would be a good idea. If we go with the MbUnit style syntax, I don't see how we can support 3rd party assertion/mocking libraries. They are test framework independent and won't be able to use any special NUnit hooks. It will also make a difference to how failed tests appear in coverage based test runners (such as NCrunch).

you already have a separate issue (#1737) that discusses the same alternative you are pushing here.

This is actually quite different to what I was experimenting with there. They both use lambdas, but that's as far as the similarities go. This is much simpler. 😉

[CharliePoole]

@jcansdale I created issue #1802 to discuss and decide on the syntax.

Your comment above doesn't make sense to me. If you'll explain further I would be glad to try to understand what you mean. You wrote...

If we go with the MbUnit style syntax, I don't see how we can support 3rd party assertion/mocking libraries. They are test framework independent and won't be able to use any special NUnit hooks. It will also make a difference to how failed tests appear in coverage based test runners (such as NCrunch).

We don't now exactly "support" 3rd party assertion or mocking libraries. We kind of co-exist with them but we don't have any features that are provided for them to use. So this syntax doesn't take anything away from them.

As to runners, I also don't see the problem. Runners are not (or should not be) dealing with the syntax implemented inside the nunit framework. They should be running tests and interpreting results.

The nunit console and gui runners won't need to be changed due to the syntax we implement. Of course, they will have to change to deal with any new elements we add to the test result, but that's completely independent of the syntax. I believe the same thing is true of 3rd party runners.

Still, I'd be glad to see an example of the kind of problems you foresee. Feel free to answer on the new issue.

[remcomulder]

We don't now exactly "support" 3rd party assertion or mocking libraries. We kind of co-exist with them but we don't have any features that are provided for them to use. So this syntax doesn't take anything away from them.

3rd party assertion/mocking frameworks primarily report data to test runners/frameworks using exceptions. Since all test frameworks will catch exceptions and attach them to the test results (usually as failures), this gives library authors a quick and easy way to provide functionality that will work across all test frameworks and runners with minimal testing and troubleshooting. With this convention, there isn't any need for a framework to 'support' these libraries, or these libraries to support any framework. It's just code that bubbles exceptions. Another example of this is the trapping and redirection of console/debug output. All frameworks and runners do this, so we have a standard way to report data from a test run.

If a framework were to introduce a special reporting feature which were only accessible through its own API, then it would be impossible for any of these 3rd party libraries to make use of the feature without any kind of framework-specific special handling.

I grant that this wouldn't subtract from any existing functionality provided by these libraries, but to me it seems desirable to design the feature in a way that makes it as accessible as possible.

I think it's worth considering that NUnit has been used as an inspiration for many other frameworks in the .NET space. Frameworks are generally measured by what they can provide vs NUnit. If every framework adds their own internal data capture system, we'll be adding considerable fragmentation to the bottom of the .NET testing tool stack.

[CharliePoole]

@remcomulder Yes, I understand that. In fact it has been a long-standing problem that NUnit gives no special recognition to the exceptions thrown by most mocking frameworks. It treats mocking exceptions as errors rather than failures. I made a proposal to one mocking framework author that we could provide special handling of their exceptions, but the answer was that our error/failure distinction wasn't something that mattered much to the users.

The quote your commented on is slightly out of context. I was complaining to @jcansdale that syntax does not cause the problems he is forseeing, although I'm well aware that we might do some things that would cause problems, if we aren't careful. But this rather formless angst about what we might do some time in the future is making it difficult to discuss the things that are readily resolvable and which won't cause any problems. That's why I was asking us to split up the discussion into different kinds of issues.

I agree that it would be a problem if we suddenly decided NUnit would not use exceptions to report errors, but that's not being discussed and I don't think it's implied in the proposed syntax. Do you?

Certainly, if you do, please point out the problems and let's work around them. Better yet, let's establish criteria stating what has to be able to work in the context of multiple assertions and write tests against those criteria. But ruling out implementing a desirable feature, merely because of fear that the implementation will be bad doesn't seem to me like the way to write software.

You wrote...

If a framework were to introduce a special reporting feature which were only accessible through its own API, then it would be impossible for any of these 3rd party libraries to make use of the feature without any kind of framework-specific special handling.

Again, I have to say that nobody is proposing such a feature - or at least I'm not. Again I ask: do you see some such API as being implied in the syntax we are discussing? If so, can we avoid it?

BTW, to the extent this is about reporting format (XML) let's continue here. If it's about syntax, let's move over to issue #1802 where the syntax I propose is described and where, hopefully, others will be proposing alternatives.

[remcomulder]

I think that perhaps the issue I'm trying to raise here isn't really fitting so well in the categorisation between syntax vs runner reporting. I apologise if I'm causing chaos in the wrong thread by bringing it up here :)

My central concern is that if I were to write my own assertion library, how would I report my assertion results to NUnit? Syntax completely aside, would this require interaction with the NUnit libraries?

Also, when assertion results are reported to the test runner, are there any plans to report non-failure cases (such as True assertions) in the XML?

Further, is there any additional information that would need to be reported with the assertion results beyond a simple message and stack trace?

I ask these questions because it seems to me that using AggregateException (or a custom substitute exception with .ToString() override for pre-.NET 4.0) would allow us to avoid needing to make any changes to the NUnit API and would still give us compatibility with existing tooling. It would allow this feature to be implemented in such a way that this thread's original question wouldn't need an answer.

But anyway, my answer to the original question hasn't changed from my first comment above. If XML changes are necessary, a config switch seems like the cleanest option to me.

[yetibrain]

For users that have xsl files for transforming NUnit xml resultfiles, it would be a small change to just add an attribute predicate to their xpath expressions. So we might also consider to give the failure-tag an attribute, telling wether it contains a single message- and stack-trace subtag or multiple ones. I thought of something like this:

<failure mode="single">
   <message>here goes the msg<message>
   <stack-trace><![CDATA at ... ]]></stack-trace>
</failure>

For multiple results we could do this:

<failure mode="multiple">
   <message>here goes the first msg<message>
   <stack-trace><![CDATA first at ... ]]></stack-trace>
   <message>here goes the second msg<message>
   <stack-trace><![CDATA second at ... ]]></stack-trace>
</failure>

If existing xsl files just mind the mode="single" their transformation should still work. Another question is if the count of failures in the test-results root tag counts multiple failures as one or counts each single failure as one failure.

[CharliePoole]

@remcomulder No, I don't think your comment is causing chaos here. You're only joining in the existing confusion :-) If your concern is a real issue, I think somebody should write an issue about it. At least, to my mind, it doesn't seem to have much to do withMultipleAssert. I'm willing to be shown I'm wrong, however.

Currently, if you write your own assertion library, you have three choices:

1. Reference nunit.framework and translate your own syntax to NUnit Asserts.
2. Reference the nunit framework and use NUnit's own exceptions to report a failure.
3. Throw some other exception, causing your failure to be treated as an error by NUnit. This would include use of AggregateException, although I don't now whether anybody is using it.

Under NUnit V2, there was a fourth option, no longer available:
4. Throw an exception of your own with the same FullName as NUnit's AssertionException.
The last is no longer available because of internal changes. It could be made available again if somebody really needed it.

I've never considered the use of Exceptions as an API, but I suppose it is now that you mention it. It's not, however, an API I had been thinking of changing.

Now, if we hypothesize a custom assertion framework that uses AggregateException to report multiple errors at once, we won't handle such a thing correctly. In fact, when I spiked multiple assertions, that's approximately what I did, although I used a custom message rather than AggregateException itself. That's when I decided that we needed a way to report multiple failures more cleanly, which is what this issue is about.

We may, in fact, want to implement better handling of AggregateExceptions thrown by other people, but that doesn't require us to use them internally ourselves. Exceptions aren't supposed to make it out of the nunit framework anyway, so they are an really only an internal implementation detail for most purposes.

SO, bottom line, this is an issue I had not considered. It's conceivable that something will come up in code review that could effect third-party assertion frameworks. We'll have to watch for that. In particular, it would be nice if such frameworks were able to participate in the multiple assertion feature and we should document how to do that. We're a bit limited there, since we don't know which of the three approaches I listed are being taken by different frameworks. As a guess, I'd say those that are already "broken" (those whose failures are treated as errors) will continue to be broken and those that integrate with the framework will be OK. We'd do better if one of those authors were to write an issue.

[CharliePoole]

@yetibrain If this format were solely intended to support multiple failures I'd be OK with an extension to the <failure> element. But, as listed in the original description, it already has three anticipated uses:

• Multiple Asserts - so that multiple failures can be reported
• Warning-level Asserts - so that both warnings and any eventual failure can be reported
• General assertion reporting - so that all asserts can be reported

That's why I'd rather replace <failure> with something like <asserts>, which could eventually contain failures, warnings and even successful results.

Obviously, <failure> would need to be retained for compatibility, at least for a while. Legacy runners would continue to process it. Runners that know about the new element would be able to use it instead, if present.

[yetibrain]

@CharliePoole I agree, <asserts> tag is to be preferred.

[CharliePoole]

@nunit/core-team @nunit/contributors
I'd like to bring this design phase to an end. I have posted an update to the specification at https://github.com/nunit/docs/wiki/Multiple-Asserts-Spec containing a description of the file changes to be made in its first section. I'll allow a few days for comment before moving to implementation.

[remcomulder]

@CharliePoole Thanks for drafting this up and for being so inclusive in the design for this feature. From my side, I don't see any issues with the new design. As expected, I'll need to make some modifications to NCrunch to enable the new feature. It would be great if you could ping when you have everything ready for integration.

I'm quite interested in the idea of also exchanging details on passed assertions back to the test runner. This is a really cool concept. I could potentially make use of this data to show green checkmarks in the NCrunch gutter marking out where assertions have passed ... I think this would be a very popular feature. Do you also plan to capture stack traces for assertions that have passed?

[CharliePoole]

@remcomulder Thanks for the comments.

Regarding passed assertions, it's really only at the idea stage and is fairly futuristic. However, what seems reasonable to me is to simply capture the location of the passing assert... just a single line of the stacktrace.

[remcomulder]

@CharliePoole Is it possible to capture a single line without pulling up the whole trace? The whole trace would actually be really useful for me, as I'm sure users would like to be able to see the checkmarks projected over the whole call stack in the same way as the red Xs. Anyway, understanding that it's a future feature, I guess this is something we could work out later.

[CharliePoole]

@remcomulder Since we already truncate the call stack it probably doesn't matter which way we go in terms of space used. The stack trace is usually rather short for assertions anyway since it doesn't normally include the frames that are below the test method itself.

[remcomulder]

@CharliePoole That's true. I was thinking about handling custom assertion methods, where the actual assertion may be nested a little deeper. Anyway, I'll place my vote for having the full stack trace exposed. Since it's collected anyway, we might as well use it :)

[jcansdale]

@remcomulder,

Is it possible to capture a single line without pulling up the whole trace?

You can use new StackFrame(1, true) in the full .NET framework, but this will only be exposed in .NET Standard 2.0 and later. Currently in .NET Core 1.0, we would need to use Environment.StackTrace (which does appear to include file/line info).

[CharliePoole]

Taking this out of design and queuing it up to work on.

[CharliePoole]

While working on the implementation of #391 I realized that we resolved the question of a new, enhanced format using <assertion> elements but left a couple of things are unresolved regarding compatibility with existing usage, so I'm re-opening it to discuss those points a bit further.

Following the specification, my working code currently concatenates all the messages from all the asserts that failed into a common failure message, like this...

1) Failed : NUnit.Framework.Assertions.Tests.AssertMultipleTests.MultipleAssertFailureDemo
  Multiple Assert block had 2 failure(s).

  1)   RealPart
  Expected: 5.0d
  But was:  5.2000000000000002d


  2)   ImaginaryPart
  Expected: 4.2000000000000002d
  But was:  3.8999999999999999d



at NUnit.Framework.Assertions.Tests.AssertMultipleTests.MultipleAssertFailureDemo() in D:\Dev\NUnit\nunit-3.0\src\NUnitFramework\tests\Assertions\AssertMultipleTests.cs:line 94

The message is missing individual stack traces for each failure, and only shows the location of the Assert.Multiple() block that failed. For a simple example, it gives enough information to find the failures but for nested blocks or asserts in called methods it is not sufficient. In the later case, the actual assert may be in a different file.

One way to resolve this would be to include individual stack traces in the message. This has the drawback that the individual stack traces could only be extracted by parsing the message. It makes it more difficult for runners to treat the embedded stack trace in any special way, such as marking the exact point of failure in the source code.

A simple approach comes to mind, but I'm not sure about the impact on compatibility. It would be very easy to include multiple <failure> elements in the XML result, either in lieu of or in addition to the new <assertion> elements. It strikes me that this would allow some runners to continue to work without change. Others - those looking for a single <failure> element would have to be revised, but it sounds like a relatively easy change.

@remcomulder @jcansdale @nunit/core-team Which would you prefer: stack traces embedded in the message or multiple failure elements?

A further question remains after we decide on the best format for compatibility. Should we introduce a new named format for the xml result? If so, when should we do it and what should we call it.

My thinking is that we can introduce a new format when adding new elements. We must introduce a new format when removing elements that people depend on or when we make any other incompatible changes.

Should we consider having a new format called either nunit4 or nunit3x? Such a format could be entirely incompatible. We would document it as being experimental at this time and therefore subject to change. Eventually, possibly with NUnit 4.0, it would become the standard format. For now, you would only get it if you asked for it.

Thoughts on this?

[remcomulder]

I think that it should be possible to design the new format in a way that allows existing runners to keep functioning, while giving space for new runners to easily plug into the format.

For me, the ideal way to handle an existing runner (which has no knowledge of multiple results/assertions) would be to concatenate the results together in the same way as you've described, and use the stack trace from the beginning of the Assert.Multiple call as the data for the stack-trace element inside the result XML. In this way, existing runners could still give meaningful results without horribly bugging out. New runners would also then have the stack trace for the start of Assert.Multiple in case they wanted to use this for other things.

Adding extra elements with a different name inside the test result XML normally shouldn't interfere with existing runners, so I would hope it would be safe to simply add extra failure elements to the test result XML. Perhaps something like this:

<test-case> 
  <failure>
    <message>Failure1\r\nFailure2</message>
    <stack-trace>stack trace of assert.multiple</stack-trace>
  </failure>
  <failures>
    <failure>
      <message>Failure1</message>
      <stack-trace>stack trace of failure1</stack-trace>
    </failure>
    <failure>
      <message>Failure2</message>
      <stack-trace>stack trace of failure2</stack-trace>
    </failure>
  </failures>
</test-case>

In this way, we'd have some level of backwards compatibility while still surfacing the necessary information for new runners. We also wouldn't need to do any crazy version detection or add a major increment to NUnit, as runners that support this format could just look for the data and use it if it is there.

[rprouse]

I like @remcomulder's suggestion. I am reluctant to introduce a new format. NUnit 3 has been RTM for a couple of years now and in pre-release for a very long time, but many third party tools are just catching up and releasing support. I don't think we want to put our users through that again if we can help it.

[CharliePoole]

@remcomulder I don't think any of the options we are discussing require version detection - crazy or uncrazy. Elements are either present or absent.

If I am understanding you correctly, your proposal would include the message for each failure in the runtime three times: once under <assertion>, once under <failures> and once concatenated into the main failure message. Is that what you are saying?

Or are you suggesting we roll back the already implemented change to use an <assertions> element and substitute <failures> for it?

@rprouse same questions: which interpretation of @remcomulder are you agreeing with. 😃

As to the user of <failures> rather than <assertions> what will we do in the future if we have warnings? Where will successful assertion results go? It seems to me that one of the biggest drawbacks of our current schema is that we have entirely different ways of showing the message and stack-trace associated with different outcomes.

Aside from that, a <failures> node is no less of a change than an <assertions> node.

I did wonder if having multiple top level <failure> nodes was a possibility. If I'm using an xpath of "test-case/failure", then I'll get multiple failures that way. OTOH, if I'm programatically asking for a single failure node, I may get an error and at best I'll only process one of them.

[remcomulder]

@CharliePoole Sorry, I don't think I articulated this very well. I wasn't trying to replace the 'assertions' approach with 'failures', rather just trying to give an example of how the newly formatted assertion results could be kept separate from the existing reporting system to maintain backwards compatibility. Trying to explain (or understand) an XML structure without examples seems to be quite hard to do.

I don't see any reason why the 'assertions' element couldn't exist containing all the data as was originally discussed, provided this was done so without disturbing the existing 'failure' element which should ideally aggregate the results from the assertions.

[CharliePoole]

@remcomulder Good point about showing the XML examples. I'll do the same...

Current structure is...

<test-case> 
  <failure>
    <message>There were 2 Failures:\r\n  1) Failure1\r\n  2) Failure2</message>
    <stack-trace>stack trace of assert.multiple</stack-trace>
  </failure>
  <assertions>
    <assertion status="Failed">
      <message>Failure1</message>
      <stack-trace>stack trace of failure1</stack-trace>
    </assertion>
    <assertion status="Passed"> <!-- Future Extension -->
      <message>passing message</message>
      <stack-trace>stack trace of passing assertion</stack-trace>
    </assertion>
    <assertion status="Failed">
      <message>Failure2</message>
      <stack-trace>stack trace of failure2</stack-trace>
    </assertion>
  </failures>
</test-case>

I was wondering if we could get away with

<test-case> 
  <failure>
    <message>Failure1</message>
    <stack-trace>stack trace of failure1</stack-trace>
  </failure>
  <failure>
    <message>Failure2</message>
    <stack-trace>stack trace of failure2</stack-trace>
  </failure>

  <!-- The remainder as above -->

</test-case>

Am I wrong to think this would actually work for many processing programs?

[remcomulder]

@CharliePoole Thanks for the example! This is so much clearer now :)

I think that multiple 'failure' elements would be a risk. We would then be making the assumption that runners are always accepting the first element in the set when processing results. Under NCrunch, this is indeed the case, but you never know when people might be using .Single() instead of .First().

Also, when this XML is processed with an older runner, we would ideally still want to show the results of all failures rather than just the first one.

For these reasons, I think that the current structure (first option) you've shown above would be the best one for compatibility. Given this as the implementation choice, I'd expect that existing runners would continue to work with the new structure while we could modify new runners to mine the 'assertion' elements for the more detailed data.

[CharliePoole]

@remcomulder You're right. The fact that it would only fail for some programs doesn't make it a viable solution. I'll stick with what we have.

@rprouse WRT introducing API changes, we decided that it was best to do it sooner - through a continuous dev build - rather than later. I'm just wondering if that wouldn't be the right approach for the XML format as well - having some format that dropped the "compatibility" parts we are adding for changes. Then people could see what's coming in the future.

[CharliePoole]

With all that, I'm ready to re-close this without further changes at the moment. Any future changes to the <assertion> element will be additions, and so compatible.

[rprouse]

Looking at your XML helps, thanks. I agree and think we should close for now.