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?
To be completely clear, maybe WithSite.

It's a bit redundant when used with an enum constant but less so when used
with a variable.
 On Oct 15, 2014 3:43 AM, "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 had the same reaction. I was unsure when I first looked at it, but it
> seemed ok the more I looked at it. I think we should go with first
> impressions though. I was a actually lying awake this morning trying to
> think of a better word to use . With works. I was also thinking about At or
> From. ResultState.Error.At( FailureSite.TearDown)?
> … <#1491367e0a8dd71b_>
> On 2014-10-14 11:48 PM, "CharliePoole" <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) 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 …
> <#14911eb82ebcbf5c_> 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>. — Reply to this
> email directly or view it on GitHub <
> https://github.com/nunit/nunit/pull/263/files#r18873533>.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/nunit/nunit/pull/263/files#r18886104>.
>