Skip to content
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

Adding NUnit MultipleAssertException to default fail exceptions #501

Merged
merged 1 commit into from
Jun 2, 2024

Conversation

algirdasN
Copy link
Contributor

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
Copy link

CLAassistant commented May 13, 2024

CLA assistant check
All committers have signed the CLA.

@delatrie
Copy link
Contributor

delatrie commented Jun 2, 2024

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.

@delatrie delatrie merged commit d0c7a23 into allure-framework:main Jun 2, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants