Relax connection stop checks in tests to reduce flakiness #2915
Conversation
| } | ||
|
|
||
| mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny<string>()), Times.Once()); | ||
| mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny<string>()), Times.AtMostOnce()); |
There was a problem hiding this comment.
Is there a point in checking these at all if we allow 0 and 1? Maybe we should just remove them entirely. I suppose we are making sure ConnectionStop is never called more than once still.
There was a problem hiding this comment.
There's almost no point other than to remind us that there was once a useful assertion here. Be extra careful not to apply these changes to release/2.2 or master when we merge.
|
Background please. What is the problem and how does this fix it? This is 2.1 only and won't be merged to 2.2 or master, correct? |
|
Most of the information is in the issue. There are test failures in 2.1 which were fixed in 2.2 and master via changes in #2646. However, @Eilon has asked to fix the tests without back-porting src changes since product changes for 2.1 are unlikely to be approved by shiproom unless it was reported by customers. |
|
Yup. |
| } | ||
|
|
||
| mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny<string>()), Times.Once()); | ||
| mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny<string>()), Times.AtMostOnce()); |
There was a problem hiding this comment.
There's almost no point other than to remind us that there was once a useful assertion here. Be extra careful not to apply these changes to release/2.2 or master when we merge.
Addresses #2750. This will reduce test coverage and I'm modifying similar tests preemptively since they could probably fail in the same pattern.