#104 Fix test instability related to timers #105
Conversation
upgrade to NUnit 4 and use Assert.That syntax everywhere added constraints to verify (no) timeout: Does.Timeout and Does.NoTimeout - verify exception throwed - verfiy error message - verify duration Refactored outcommented test to use [Ignore] attribute Single place to read resource path from assembly: BaseTest.EmptyHtmlPath
| } | ||
|
|
||
| var elapsedTime = DateTime.Now.Subtract(beforeCall); | ||
| return new ConstraintResult(this, elapsedTime, elapsedTime < maximumDelay && elapsedTime >= (minimumDelay ?? TimeSpan.Zero)); |
There was a problem hiding this comment.
probably most people will naturally read "maximum" as "maximum possible", i.e. "inclusive the maximum as a valid value among the range"... Probably would be better then to use <= over <...
| return new ConstraintResult(this, elapsedTime, elapsedTime < maximumDelay && elapsedTime >= (minimumDelay ?? TimeSpan.Zero)); | ||
| } | ||
|
|
||
| public override string Description => $"Should not timeout" + (minimumDelay.HasValue ? $" or pass in less then {minimumDelay}" : ""); |
There was a problem hiding this comment.
Shouldn't we mention tyhe maximumDelay value in tyhe description too?
I propose the following error message:
public override string Description => $"Should not timeout" + (minimumDelay.HasValue ? $" and pass within: {minimumDelay} < delay <= {maximumDelay}" : "");
If the actual comparison was tuned to <= then description should be tuned correspondingly...
| internal class TimeoutConstraint(string errorMessage) : Constraint | ||
| { | ||
| private readonly string errorMessage = $$""" | ||
| Timed out after {{Configuration.Timeout}}s, while waiting for: |
There was a problem hiding this comment.
Heh:) Tightly coupled to the "global variable" that is also might be not obvious from the usage:)
the accuracyDelta below is also kind of "hardcoded"... I would at least inject these values as dependencies, then use same defaults, but at least having an option to parametrize if needed...
But OK, we can keep it as it is for now... and later I can rafactor these things for this constraint...
|
Hm, let me actually do small adjustments mentioned above on my own and finalize this PR... |
|
@wjgerritsen-0001 Thank you so much for so enormous efforts! Great job! |
use global usings to reduce the number of usings in each file
upgrade to NUnit 4 and use Assert.That syntax everywhere added constraints to verify (no) timeout: Does.Timeout and Does.NoTimeout