Uh oh!
There was an error while loading. Please reload this page.
improve accuracy of Expect100Continue_WaitsExpectedPeriodOfTimeBeforeSendingContent test - #44053
Conversation
…SendingContent test
ghost
commented
Oct 30, 2020
Tagging subscribers to this area: @dotnet/ncl |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
wfurt
commented
Oct 31, 2020
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
wfurt
commented
Nov 3, 2020
all failures seems unrelated. running outer loop one more time to be sure. |
wfurt
commented
Nov 3, 2020
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| Assert.InRange(sw.Elapsed, delay - TimeSpan.FromSeconds(.5), delay * 20); // arbitrary wiggle room | ||
| long elapsed = content.Ticks - start; | ||
| Assert.True(elapsed >= delay.TotalMilliseconds); |
There was a problem hiding this comment.
You might want to have a small amount of leeway here in checking that elapsed >= delay, just in case of timing weirdness -- looks like the test previously had 0.5s of leeway?
If it's reliable as is, then maybe no leeway is necessary.
geoffkizer
left a comment
There was a problem hiding this comment.
The test logic is definitely better this way, thanks.
From the name of the test, I assume we are trying to verify that provided
Expect100ContinueTimeoutis respected e.g. we wait longer than default 1s.Right now the test does it be measuring whole request/response and verifying duration within arbitrary time range.
With delay of 3s and fudge of 20 we expect everything to finish within 1 minute. From test failures are are sometimes over that - with max duration ~ 1:30 -1:40.
This PR is changing the test logic. Instead of measuring the whole duration, I measure delay between start and sending content e.g. call to SerializeToStreamAsync(). It should be at least expected delay - and could be more depending on system load and other environmental conditions. With default 1s, that should be sufficient verification that the delay actually works.
I originally used Stopwatch and I saw occasion failures when elapsed time would be slightly shorter e.g. 2995-2999.
I solved by using Environment.TickCount64 instead as it seems to use semi coarse base as Timer class.
fixes#41530
fixes#41953