Throws and Delayed (.After) Constraints do not cooperate, resulting in incorrectly failing test

[seesharks]

NUnit Version: 3.13.3 (latest stable as of this posting)

Goal: I am attempting to repeatedly check a method to Assert that it eventually throws an Exception within a certain amount of time. (Specifically, looking to get a 401 back from a webcall after a delete, where the deletion does not always occur instantly).

Approach: I would like to do this by using a constraint of the form Throws.<...>.After.<...>.

Issue: Unfortunately, the combination of these constraints results in the test failing due to the thrown Exception, rather than passing due to the Assert that the Exception will be thrown. It appears that that nunit gets snagged on the Delayed constraint, never making it to the Throws constraint to see that the Exception was actually the expected outcome.

Note that .After works fine with other constraints, such as Is. Similarly, Throws works fine when I leave off the .After constraint, simply checking that the method throws the first time it is called.

Example: The commented-out counting code was implemented to simulate the repeated calls, e.g., 4 calls before the expected error is thrown. But the presence or absence of that code has no impact on the outcome.

    [Test]
    public void TestThrowsAfter()
    {
      Assert.That(() => ThrowingMethod(), Throws.TypeOf<ArgumentException>().After(5).Seconds.PollEvery(500));
    }

    private int count;
    private int ThrowingMethod()
    {
      Thread.Sleep(50);
      // Uncommenting this code has no effect on the outcome:
      //if (count < 5)
      //{
      //  return ++count;
      //}
      throw new ArgumentException("test");
    }

Example Result: When I run the above, I get a test failure, with the following readout:

System.ArgumentException : test
   at Tests.ThrowingMethod() in <throw line>
   at Tests.<TestThrowsAfter>b__48_0() in <Assert.That line>
   at NUnit.Framework.Constraints.DelayedConstraint.InvokeDelegate[T](ActualValueDelegate`1 del)
   at NUnit.Framework.Constraints.DelayedConstraint.ApplyTo[TActual](ActualValueDelegate`1 del)
   at NUnit.Framework.Assert.That[TActual](ActualValueDelegate`1 del, IResolveConstraint expr, String message, Object[] args)
   at NUnit.Framework.Assert.That[TActual](ActualValueDelegate`1 del, IResolveConstraint expr)
   at Tests.TestThrowsAfter() in <Assert.That line>

[OsirisTerje]

I've added the repro code above with a few smaller changes. https://github.com/nunit/nunit.issues/tree/main/Issue4281

The TestThrows test (without the delayed constraint), works as it should as @seesharks states.

The TestThrowsAfter does not, and just returns the exception, it is, again as @seesharks points out, not being handled by the That method.

Also interesting is that the Polling is weird, they come pairwise as two and two nearly simultaneously, and not with the expected 500mS interval. Between 1 and 2 and between 3 and 4 it is correct.

[OsirisTerje]

To test the Is with After, added some more code, and now, again as @seesharks says, the That correctly catches the result, but the PollEvery has the same strange behaviour. We should have seen 4 iterations, but we get 8,

[mikkelbu]

Regarding the strange behaviour, then I think we have seen it before. See #2841 (comment) . I think I might have a branch for this somewhere

[OsirisTerje]

@mikkelbu Awesome! It also seem that you actually have the fix for that in #2841. With the repro here, it can be verified. And adding a test for it in NUnit should be doable too.

To debug it, I added this (but I can skip this now, since you found the cure):

        private int _pollCount;
        
        [Test]
        public void VerifyPolling()
        {
            _pollCount = 0;
            Assert.That(() => PollCount(), Is.EqualTo(4).After(1000, 250));
        }

        private int PollCount() =>  return _pollCount++;

I am bad at counting, so the correct answer should be either 3, 4 or 5. Currently it is 7, which obviously is wrong.
The test I have in the repro also confirms it is actually calling twice, since the timestamps are the same.

When #2841 is fixed, we can take a look at the constraint mess that also seems to happen here in this issue with the Throws.

[OsirisTerje]

@mikkelbu Added a PR #4488 with your fix above and a test :-) Then #2841 is fixed, and we can concentrate on this one.