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

Add built-in support to exit an Assert.Multiple() block early #4461

Closed
stevenaw opened this issue Aug 17, 2023 · 7 comments
Closed

Add built-in support to exit an Assert.Multiple() block early #4461

stevenaw opened this issue Aug 17, 2023 · 7 comments

Comments

@stevenaw
Copy link
Member

As discussed in #4451

It may be helpful to have a built-in way to exit an Assert.Multiple() block early. Currently the only ways are:

  • Throw an exception
    • This will cause the multiple to fail, which may not be desired
  • Explicit return statement in the lambda
    • This has the obvious limitation it must be in the lambda body directly
    • This will not exit nested Assert.Multiple() in a single command

Some ideas discussed in #4451 are:

  • Adding a new method such as Assert.MultipleExit() or Assert.FailFast()
  • Adding a parameter to an existing method
  • Something else?
    • A MultipleAssertionContext passed into the lambda by the framework as an argument for the lambda
@stevenaw stevenaw changed the title Add first-class support to exit a Assert.Multiple() block early Add built-in support to exit an Assert.Multiple() block early Aug 17, 2023
@manfred-brands
Copy link
Member

One more thing to consider, it the FailFast exiting only one or all levels of a nested Assert.Multiple().
I suggest only one level as the code at that point doesn't know about higher levels.

@stevenaw
Copy link
Member Author

True. If we use a method-based approach, I wonder if we'd want to allow that to be configurable.

Assert.FailFast(AssertMultipleScope.Immediate);
Assert.FailFast(AssertMultipleScope.All);

To me the name FailFast() implies it should do all scopes but I agree that we'd likely want to default behaviour of this feature to only be one scope so perhaps that's another argument against the name FailFast() for this

@stevenaw
Copy link
Member Author

I find the idea of something like this intriguing too:

public class MultipleAssertContext {
  public MultipleAssertContext? ParentContext { get; }
  public int FailureCount { get; }
  public int TotalFailureCount { get; }
  public void Exit();
}

and then:

Assert.Multiple((context) => {
  // Lots of complicated assertions

  if (arbitraryCondition)
  {
    context.Exit();
  }

  Assert.Fail("This doesn't run");
});

@manfred-brands
Copy link
Member

I don't think we should add a parameter, potentially we can add something to TestExecutionContext instead,

There is currently no hierarchy of MultipleAssertContext, there is only an (internal) MultipleAssertLevel property.

I don't see many test themselves care about FailureCount.
This is already available as TestContext.CurrentContext.CurrentResult.PendingFailures

@stevenaw
Copy link
Member Author

I could see TestExecutionContext making sense for this. Did you have anything in particular in mind?

@stevenaw
Copy link
Member Author

I think the introduction of the AssertionScope also makes this a bit easier to exit the test method early based on arbitrary criteria as the individual assertions no longer need to occur in a callback.

@manfred-brands Reaching out as you're the other to have been involved in discussion here. I'm inclined to close this, but feel free to reopen if you feel otherwise

@stevenaw stevenaw closed this as not planned Won't fix, can't repro, duplicate, stale Jul 31, 2024
@stevenaw stevenaw added this to the Closed Without Action milestone Jul 31, 2024
@manfred-brands
Copy link
Member

@stevenaw I agree on closing this. If early exit is wanted, why are they combined in a single Assert.Multiple?
It is easy to split them if the 2nd is not independent.

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

2 participants