allure-framework / allure-csharp

Allure integrations for C# test frameworks
https://allurereport.org/
Apache License 2.0
106 stars 65 forks source link

Adding NUnit MultipleAssertException to default fail exceptions #501

Closed algirdasN closed 3 months ago

algirdasN commented 4 months ago

Context

NUnit provides a method to perform multiple assertions without interrupting by failure. If any failures occur, a MultipleAssertException is thrown after all assertions have finished. Currently this exception is not included in default fail exceptions and this causes Allure to mark such test failures as broken tests.

Read more about multiple asserts here - https://docs.nunit.org/articles/nunit/writing-tests/assertions/multiple-asserts.html

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

delatrie commented 3 months ago

Hi, @algirdasN , thank you for your contribution! You're correct. Since MultipleAssertException is clearly assertion-related and isn't derived from AssertionException, we want it to be included in the default failExceptions.

But I want to clarify a little bit about how all that works with NUnit.

and this causes Allure to mark such test failures as broken tests.

That's not entirely true. We don't use failExceptions to compute a test's status in Allure NUnit, partially because of API limitations and partially because NUnit separates failed assertions from unhandled exceptions unrelated to assertions. The documentation will be corrected soon.

As long as the test contains at least one failed assertion and doesn't throw anything besides exceptions originating from Assert, the test will be failed.

But what is affected by that property are steps and fixtures. And that's where lacking MultipleAssertException leads to an incorrect status being assigned to the step or fixture.

To illustrate all this, let's consider the following test:

[AllureNUnit]
class Pr501Tests
{
    [Test]
    public void MultipleAssertTest()
    {
        AllureApi.Step(
            "Step with multiple assertions",
            () => Assert.Multiple(() => {
                Assert.That(1 + 1, Is.EqualTo(3));
                Assert.That(1 + 1, Is.EqualTo(1));
            })
        );
    }
}

It's shown in the report as follows:

Failed test, broken step

Note the test's status is failed (which is correct), while the step's one is broken (which is not). And that's exactly what your PR is going to fix.

All those only apply to NUnit. Allure SpecFlow and Allure Reqnroll both use the general algorithm that relies on failExceptions and both will be affected by this PR.