Issue 255

Update: Ready to merge.

I'm creating this PR so we can discuss options for handling various kinds of failures that can occur in a suite. There are three places failures can occur: in OneTimeSetUp (including Before actions), in a child test or in OneTimeTearDown (including After actions).

In this commit, I have created separate failure states for each of those alternatives: SetUpFailure, ChildFailure and TearDownFailure. I made corresponding changes to the reporting so that it continues to work correctly and was able to do away with checking for a specific message text to recognize that a failure was due to a problem in a child test.

However, I'm not entirely happy. Other things can also happen in these three places: unexpected exceptions, ignoring the test failure of an Assume. So do we take every outcome and multiply it by three?
Do we need SetUpError, SetUpInconclusive, etc?

In NUnit 2.x, we had the FailureSite enum and the FailureSite property of TestResult. The enum had values of SetUp, Test, TearDown, Child and Parent. Instead of proliferating result states, we could add a new Site property to ResultState to indicate where any non-Success result originated. Assert.Ignore in setup would give a ResultState.Ignored, with the Site set to SetUp. Would this lead to clearer code than the present approach of adding entirely new ResultStates?
Yes, I was a bit unsure of it and then it started feeling OK. In
retrospect, it may be an example of code that's easy to write (when you
know about the overload) but hard to read (when you don't).

Here's an idea that comes to mind...

    public readonly static ResultState TearDownError =
ResultState.Error.With(FailureSite.TearDown);

What do you think?

Charlie

On Tue, Oct 14, 2014 at 6:49 PM, Rob Prouse <notifications@github.com>
wrote:

> In src/NUnitFramework/framework/Interfaces/ResultState.cs:
>
> > +        /// A suite had an unexpected exception in its OneTimeDown
> > +        /// </summary>
> > +        public readonly static ResultState TearDownError = ResultState.Error + FailureSite.TearDown;
> > +
> > +        #endregion
> > +
> > +        #region Operator Override
> > +
> > +        /// <summary>
> > +        /// Return a new ResultState that uses the Status and Label of the
> > +        /// curent ResultState plus a specified site.
> > +        /// </summary>
> > +        /// <param name="result"></param>
> > +        /// <param name="site"></param>
> > +        /// <returns></returns>
> > +        public static ResultState operator +(ResultState result, FailureSite site)
>
> I'm a bit uncomfortable with this type of operator overload. When I read
> the static ResultStates above the semantics confused me until I read the
> operator overload. It is an interesting way to merge the two pieces of
> information, but I wonder if there is a clearer pattern for accomplishing
> the same thing?
>
> If you don't mind, I would like to think on this one a bit.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/nunit/nunit/pull/263/files#r18871228>.
>