Skip to content

Multiple Assertions #391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
circa1741 opened this issue Nov 27, 2014 · 46 comments
Closed

Multiple Assertions #391

circa1741 opened this issue Nov 27, 2014 · 46 comments

Comments

@circa1741
Copy link

circa1741 commented Nov 27, 2014

May I ask if there is a plan to have something similar to MbUnit's Multiple Assertions capability?

This might provide some insight:
https://code.google.com/p/mb-unit/source/browse/trunk/v3/src/MbUnit/MbUnit/Framework/Assert.cs?r=1570#282

Evaluates a block of code that contains multiple related assertions.

While the action delegate runs, the behavior of assertions is change such that
failures are captured but do not cause a
to be throw. When the delegate returns, the previous assertion failure behavior
is restored and any captured assertion failures are reported. The net effect
of this change is that the test can continue to run even after an assertion failure
occurs which can help to provide more information about the problem.

A multiple assertion block is useful for verifying the state of a single
component with many parts that require several assertions to check.
This feature can accelerate debugging because more diagnostic information
become available at once.

Also:
https://code.google.com/p/mb-unit/source/browse/trunk/v3/src/MbUnit/MbUnit/Framework/MultipleAssertsAttribute.cs?r=1570

Runs the test as if it were surrounded by so that
multiple assertion failures within the test are tolerated.

When an assertion failure occurs, it is reported but the test is allowed to proceed
until it completes or throws an unhandled exception. When the test finishes, it will
still be marked as failed, as usual.

Partial specs as entered below by @CharliePoole

There are a number of things needed here and I guess it's time to make a list! I may end up breaking some of these out into separate issues, but for the moment let's just stick with bullet points.

  • We will have to redesign the xml result format to support multiple results per test case. Currently, each test can only have a single result and tests are nested inside other tests (suites). However, if each assertion can generate a separate result, we need to hold them somewhere. This might be, for example, a <results> element containing one or more <result> elements and superseding some existing elements.
  • The internal framework TestResult Type will need to hold multiple results and generate xml as designed in the first bullet.
  • Clients and the engine will need a way to recognize which format is being used.
  • Clients and the engine may need a way to control which format gets used.
  • We'll have to firm up the syntax we intend to support. This is the part we have mostly discussed here, so I think it's pretty ready.
  • We need to introduce a mechanism for reporting results during a test without generating an exception, which would, of course, stop the test.
  • Finally, we have to implement the syntax for multiple assertions.

The preliminary part of this, supporting multiple results per test, will actually be useful for three different features we would like to have:

  1. Multiple Assertions (this issue)
  2. Warning Assertions - because allowing continuation after a warning implies multiple results
  3. Reporting passed assertions.

If anyone wants to pull out a part of this and work on it, let me know.

@rprouse
Copy link
Member

rprouse commented Nov 27, 2014

I'll throw my vote in here 👍

Mulitple asserts make unit testing purists cringe, but we use them in some long running integration tests. As you said, they can give you a lot more information about failing integration tests.

@CharliePoole
Copy link
Member

Me too. I guess Rob's comment about purists refers to the "Single Assert"
rule, but I have always taught that a test is best written to contain a
single "logical" assert. That is, it may not be possible to write a single
Assert in the framework you are using to make a single logical test. For
example, you are testing an address, which exists in multiple fields. You
may want to write a single test that checks all those fields using separate
asserts. It would be handly to see all those results, not just the first
one that happens to fail. So I don't rule them out even in unit tests.

Let's use this issue to figure out the syntax we want to support. Some
options I have already tried experimentally...

  1. We have experimentally used Assert.Group / Assert.UnGroup to surround a
    set of assertions intended to operate in the way you describe. It's very
    much a procedural approach, but has the benefit (?) of working in .NET 2.0.
    It introduces the possibility of nesting and how to deal with it as well as
    failure to terminate and what the impact of that is.
  2. A separate "verb" replacing Assert, which simply records its error and
    goes on. For example, Check.That(...). Failure is only reported when an
    actual Assert fails or at the end of the test. It could be supplemented
    with a variation like Check.Report(), which would cause failure earlier.
    This is also rather procedural in nature and I abandoned it. * Included
    here for completeness.
  3. A using syntax, creating an object, which changes the way asserts work
    for the duration of the block. This seems a lot cleaner than Add Monotouch build #1 and
    eliminates the termination problem. Nesting still has to be dealt with, but
    that's not too hard. This is my current favorite.

From what you write, I'm assuming that MbUnit uses an action delegate or
possibly a lambda to group Asserts. To move this forward, I hope you will
provide samples of code that uses the feature. Pointing to the
implementation within MbUnit is not helpful, since MbUnit uses a license
that isn't compatible with ours. Looking at their code doesn't hurt of
course, but I'm always concerned that I'll absorb too much from that look
and end up writing the same thing in NUnit without realizing it.

The key issue for all implementations is how we tell Assert that it is
operating in a different "mode". The obvious answer is to use a static
flag, since the Assert methods are all static. As you may imagine, I really
hate that idea. Maybe we can do something in the TestExecutionContext
instead.

I'd like to see us get this into 3.0 in some form or another, so let's
discuss the alternative forms. Once narrowed down, we'll be looking for a
volunteer to write a spec on the wiki.

Charlie

On Thu, Nov 27, 2014 at 7:16 AM, Rob Prouse notifications@github.com
wrote:

I'll throw my vote in here [image: 👍]

Mulitple asserts make unit testing purists cringe, but we use them in some
long running integration tests. As you said, they can give you a lot more
information about failing integration tests.


Reply to this email directly or view it on GitHub
#391 (comment).

@circa1741
Copy link
Author

(Samples taken from http://blog.bits-in-motion.com/search?q=mbunit)

There are two ways to do this in MbUnit.

Tag the method with [MultipleAsserts]:
multipleasserts

Or use the Assert.Multiple function and pass in a delegate:
assertmultiple

The MultipleAssertsAttribute is useful when UI testing a wizard and verifying its screens' default states. With the use of [MultipleAsserts], I should be able to verify the default states of the elements on screen 1, click next then verify the states on screen 2, click next, etc.

@CharliePoole
Copy link
Member

I really like both of these.Let's call it a 3.0 feature... I haven't assigned it to a milestone yet... we'll be doing a planning session after the alpha-3 release.

@CharliePoole CharliePoole added this to the 3.0Beta1 milestone Nov 30, 2014
@CharliePoole CharliePoole self-assigned this Dec 5, 2014
@CharliePoole
Copy link
Member

I've been playing around with this. Here are some thoughts:

Assertions generate a ConstraintResult. We'll need some kind of MultipleConstraintResult that will bundle a collection of failing results and report all those results at the end of the block.

The following should always terminate the test, even in a Multiple block:
Assert.Fail, Assert.Pass, Assert.Ignore, Assert.Inconclusive
Alternatively, we could consider them as invalid when appearing in a block.

The same goes for assumptions. The purpose of assumptions (Assume.That) is to prevent execution of subsequent assertions.

If we allow any of the above in a block, we need to specify what to do for each of them if they are encountered with pending failures accumulated.

If we add warnings (as is planned) to NUnit 3.0 we'll have to accumulate them as well, reporting them at the end of the test.

@rprouse
Copy link
Member

rprouse commented Dec 5, 2014

I haven't done any coding, but I was looking at this a bit too. I agree
with your conclusions about Assert.Pass, etc. I was thinking about a
different approach though. I was thinking that a multiple assertion block
would set up some sort of scoping object, then where we would throw an
exception for the failure, if the multiple assertion object is in scope, we
add the error or exception to that object instead of throwing. Then we
could throw when the object goes out of scope. I haven't fully thought it
through, but I put it out there in case it gives you ideas.

On 5 December 2014 at 02:03, CharliePoole notifications@github.com wrote:

I've been playing around with this. Here are some thoughts:

Assertions generate a ConstraintResult. We'll need some kind of
MultipleConstraintResult that will bundle a collection of failing results
and report all those results at the end of the block.

The following should always terminate the test, even in a Multiple block:
Assert.Fail, Assert.Pass, Assert.Ignore, Assert.Inconclusive
Alternatively, we could consider them as invalid when appearing in a block.

The same goes for assumptions. The purpose of assumptions (Assume.That) is
to prevent execution of subsequent assertions.

If we allow any of the above in a block, we need to specify what to do for
each of them if they are encountered with pending failures accumulated.

If we add warnings (as is planned) to NUnit 3.0 we'll have to accumulate
them as well, reporting them at the end of the test.


Reply to this email directly or view it on GitHub
#391 (comment).

Rob Prouse

I welcome VSRE emails. Learn more at http://vsre.info/

@CharliePoole
Copy link
Member

Hi Rob,

I was focused on how it should appear to users. For the implementation, we
definitely need to do what you describe.

Taking it a step further, We throw ResultExceptions in various places. It
has always been in my mind that we should centralize the reporting
somewhere. Let's call this AssertReporter for the moment... it's an
implementation detail whether it's just a static method in Assert or an
instance of an object in the context or whatever...

I'm thinking that each place where we now throw should instead report the
result to this AssertReporter... It would need to track a number of things,
including whether we were in an Assert.Multiple block. It would receive
ConstraintResults and decide what to do with them. It would save the
results or throw them depending on it's state. It would save Warning
messages, when we have them. At the end of a Multiple block (allowing for
nesting as well) it would construct a composite result and throw that. At
the end of a test, it would also bundle up warnings to be reported.

I would expect to need a small object constructed in a using statement to
control the switching in and out of Multiple status.

This centralized reporting is something we need for several other reasons
and implementing Assert.Multiple seemed to me to be the place to do it.

But getting back to how it looks to users, we're left with some
questions... They are all of the form "If we have accumulated assert
failures and we encounter X, what should we do?" The strangest one is where
X is Assert.Pass.

I think I should probably pick some answers and write a spec to be
reviewed. That will give us something more specific to talk about.
Meanwhile, I'll move on with implementing some of the infrastructure.

FYI, I'm blocked waiting for info from the developer of Mono.Addins, so I
picked this "little" thing to work on. If I get back on track with the
addins, I may set this aside, but let's get the specs resolved anyway.

Charlie

On Fri, Dec 5, 2014 at 5:56 AM, Rob Prouse notifications@github.com wrote:

I haven't done any coding, but I was looking at this a bit too. I agree
with your conclusions about Assert.Pass, etc. I was thinking about a
different approach though. I was thinking that a multiple assertion block
would set up some sort of scoping object, then where we would throw an
exception for the failure, if the multiple assertion object is in scope,
we
add the error or exception to that object instead of throwing. Then we
could throw when the object goes out of scope. I haven't fully thought it
through, but I put it out there in case it gives you ideas.

On 5 December 2014 at 02:03, CharliePoole notifications@github.com
wrote:

I've been playing around with this. Here are some thoughts:

Assertions generate a ConstraintResult. We'll need some kind of
MultipleConstraintResult that will bundle a collection of failing
results
and report all those results at the end of the block.

The following should always terminate the test, even in a Multiple
block:
Assert.Fail, Assert.Pass, Assert.Ignore, Assert.Inconclusive
Alternatively, we could consider them as invalid when appearing in a
block.

The same goes for assumptions. The purpose of assumptions (Assume.That)
is
to prevent execution of subsequent assertions.

If we allow any of the above in a block, we need to specify what to do
for
each of them if they are encountered with pending failures accumulated.

If we add warnings (as is planned) to NUnit 3.0 we'll have to accumulate
them as well, reporting them at the end of the test.


Reply to this email directly or view it on GitHub
#391 (comment).

Rob Prouse

I welcome VSRE emails. Learn more at http://vsre.info/


Reply to this email directly or view it on GitHub
#391 (comment).

@CharliePoole
Copy link
Member

CharliePoole commented Dec 5, 2014

I've posted a spec for this feature at https://github.com/nunit/docs/wiki/Multiple-Asserts-Spec

Kindly comment on it here.

@CharliePoole
Copy link
Member

I did a spike and was able to get it to work in an adhoc way by creating a message that included all the failures, with their messages and stack traces. The real solution requires making a number of changes to the how we report assertions and these should be in place first. The main pre-requisite is that TestResult should be capable of holding a list of AssertionResults and all our upstream reporting should be able to deal with that change.

I'm marking this issue as pending for now.

@CharliePoole CharliePoole modified the milestones: 3.0, 3.0Beta1 Feb 21, 2015
@CharliePoole CharliePoole removed the 3.0 label Mar 26, 2015
@svaze
Copy link

svaze commented May 13, 2015

Hey Charlie,
This feature is one of the best features for functional automated testing.
Many users out there will love to have this feature. Personally my favour for mbUnit was for the following reasons,

  1. MultipleAssert support,
  2. Parallelism and
  3. Screenshot appending/attaching capabilities to Test Log.
    These all features were not present in NUnit. However, mbUnit has now stopped any development and provides virtually no support now, so its becoming obsolete to work with.
    With NUnit 3.0, parallelism is available which will make it popular for Functional automated testing community if along with it, features like MultipleAsserts and Screenshot appending are included in it.

Many thanks for such a wonderful product.

Thanks, Shrirang Vaze

@CharliePoole
Copy link
Member

@svaze Thanks for the input. As you point out, this feature is particularly useful for functional testing. We have kept it in the plan for the 3.0 release for exactly that reason. OTOH, since it's less useful for unit testing, which has always been the core of NUnit, we have not yet given it a high priority.

The status is pending because of the internal changes needed to implement it, as described in some of the comments above. Allowing a Test to have multiple Results is a fairly big design change and potentially disruptive to the rest of NUnit. We will continue to work on this idea and try to keep it in the 3.0 release, but it's possible it will get pushed to the following release, depending on our resources. It's more important to us to actually have a 3.0 release!

The feature involving attachments has never been requested. If you would like to create an issue for it, that would be a welcome contribution. You should provide fairly complete detail on how it would work, how it would be used by tests, etc. Note that saying it should work "like mbUnit" won't really do the job for us. Only a few folks working on the project are familiar with mbUnit. If only those few can understand your request, then it's much less likely to be implemented. Fair enough?

@CharliePoole CharliePoole modified the milestones: 3.2, 3.0 Aug 23, 2015
@shortstacked
Copy link

Hey,
I have a question about this feature. I'm using NUnit.3.0.0-rc-2, and I am able to see the Assert.Multiple() method which accepts a TestDelegate, however when using this with multiple asserts the first failure causes it to break out and report.

I can see the milestone was modified but I'm unsure if that means available as a whole or only partially available as the [MultipleAssert] attribute also does not seem to be available.

Would you be able to advise if this feature is available or if the multiple assert feature is just not fully implemented yet?

Thanks.

@CharliePoole
Copy link
Member

As indicated on #391, the feature has been postponed until NUnit 3.2.

Assert.Multiple is a stub method which seems to have been committed
accidentally. All it does is execute the delegate you give it.

On Thu, Nov 12, 2015 at 1:06 AM, shortstacked notifications@github.com
wrote:

Hey,
I have a question about this feature. I'm using NUnit.3.0.0-rc-2, and I am
able to see the Assert.Multiple() method which accepts a TestDelegate,
however when using this with multiple asserts the first failure causes it
to break out and report.

I can see the milestone was modified but I'm unsure if that means
available as a whole or only partially available as the [MultipleAssert]
attribute also does not seem to be available.

Would you be able to advise if this feature is available or if the
multiple assert feature is just not fully implemented yet.

Thanks.


Reply to this email directly or view it on GitHub
#391 (comment).

@CharliePoole
Copy link
Member

Devil in the details for sure! As well as in the mind of the user who has to understand what's happening! I commented separately on #1737.

Now let's all get back to how this will look to runners!!!

@CharliePoole
Copy link
Member

As you see, I made this into an Epic. (If you don't see, you have to install ZenHub.)

I think #1743 is the key item requiring some decisions in order to move ahead.

@yetibrain
Copy link

IMHO there is no need to use an extra Attribute in order to mark a test as a multi-assert test. It's unflexible. Why not allow common Asserts along with new classes like SingleAssert? SingleAssert objects could be added to a MultiAssert object. Like this, common assert can lead to assertions as usual while several single-asserts, added to a multi-assert object fulfills the multi-Assertion Feature.

@CharliePoole
Copy link
Member

@yetibrain I agree that there's no need... but it's part of the syntax that somebody requested because it happens to match what they use in mbUnit. I see it as a marginally convenient shorthand that equates to wrapping all the code in an Assert.Multiple block.

When I made this issue into an Epic, I didn't create a separate issue for designing the syntax because I thought it had been settled for more than a year. It sounds like you would like to discuss an alternative syntax, as has also been proposed by @jcansdale in #1737. Do we need to make a new issue for the syntax? Right now, I'm focused on what the result file will look like (#1743) but I don't think more discussion of the syntax would hurt us and it's pretty much independent of the shape of the output.

@CharliePoole
Copy link
Member

After a great deal of discussion under issue #1802, now closed, the spec on the wiki has been updated:
https://github.com/nunit/docs/wiki/Multiple-Asserts-Spec

Implementation will proceed under this Epic as soon as the other sub-issues are resolved.

@CharliePoole
Copy link
Member

Cool! It turns out you can add issues from multiple repositories to an epic!

@CharliePoole
Copy link
Member

I think we have one loose end in the implementation. I'll lay it out here and we can decide how to proceed, creating another sub-issue if necessary.

The spec describes actions to take on early termination. See https://github.com/nunit/docs/wiki/Multiple-Asserts-Spec#early-termination

In each case, if there are pending failures, we are supposed to give a special message. This is not yet implemented. It's not clear whether we should list the failures in some or all of these cases.

@nunit/core-team Should we implement messages as described? Should we list info about the pending failures?

@CharliePoole
Copy link
Member

I'm going ahead and creating a separate issue, but I'm still looking for comments from @nunit/core-team and @nunit/contributors

@circa1741
Copy link
Author

I vote for implementing the special message for pending failures and listing it for all of the cases.

@CharliePoole
Copy link
Member

I created issue #1907 to handle this in nunitlite and nunit/nunit-console#136 for the console runner.

@CharliePoole
Copy link
Member

Closing the epic itself, now that all the issues are complete.

@rprouse rprouse modified the milestone: 3.7 Mar 15, 2017
johnmwright pushed a commit to johnmwright/nunit that referenced this issue Oct 28, 2019
Using new adapter related settings from runsettings of testplatform
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests