Accumulate further failures if any on AssertMultiple instead of throwing

[ghost]

Closes #4264

This implements the suggested solution. It turns out it was suitable to fix this issue. We still throw on the first fail, but any further one is only cumulated in case one appears after a catch block or in the teardown.

[mr-russ]

Is there a way to add a test to verify it will stay working in the future?
Should this get a 3.13-dev fix if it's accepted?

[ghost]

Is there a way to add a test to verify it will stay working in the future? Should this get a 3.13-dev fix if it's accepted?

I'm actually not sure how you can reliably test this: you expect an assert to fail, but lines after to be executed. I don't think I am knowledgeable enough on setting up such a test because the best I could get on my hand was expecting a failure shaped in a certain way in the console, I just don't know how you make a test for this.

As for 3.13, either works, it's a low priority issue.

[mr-russ]

I looked at how the existing AssertMultiple does its testing, and https://github.com/nunit/nunit/blob/master/src/NUnitFramework/tests/Assertions/AssertMultipleTests.cs#L124 suggests that making an explicit test that produces what you are expecting is probably the best method. Even if only run manually for checking it's reproducible and in the code and can be checked at any time.

[ghost]

There, I added an explicit test

[stevenaw]

Thanks for the quick fix you applied here @sdelarosbil . The fix itself looks good, but can we add this repro as a non-explicit test so that it may fail if a future PR accidentally undoes your bugfix?

[OsirisTerje]

@sdelarosbil Can you copy-add in the test that @stevenaw suggested? We could then merge this PR immediately after that. Nothing else seems to be missing in this PR.

[ghost]

@sdelarosbil Can you copy-add in the test that @stevenaw suggested? We could then merge this PR immediately after that. Nothing else seems to be missing in this PR.

Yes, I tried to add the test, the problem is it's not possible as far as I could see to make it not fail.

The test is designed to fail in a specific way such that something after an assert still executes. If I copy verbatim, the test fail, but in an expected way which I didn't find a way to turn it into a success. If there's something I am missing, please let me know, but I am confused how the test should be implemented to not have that happen.

[OsirisTerje]

@sdelarosbil Accepted, we'll move the testing out as a separate issue. @stevenaw or I can try to get it working.
The issue is #4331