Fix Test Watcher Timeouts and Log Checker File Reading Issues #83427
Conversation
|
Tagging subscribers to this area: @hoyosjs Issue DetailsAddresses #83298.
|
|
Please review carefully as soon as possible to fix the outer loop problems. |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| if (!File.Exists(statsCsvPath)) | ||
| { | ||
| Console.WriteLine("[XUnitLogChecker]: An error occurred. No stats csv" | ||
| + $" was found. The expected name would be '{statsCsvPath}'."); | ||
| return FAILURE; | ||
| } |
There was a problem hiding this comment.
Is this going to cause a problem for the work items that are completely filtered out (test run effectively skipped) in some of the outerloop runs? I know we just had to fix that earlier, so I want to make sure we don't break that scenario again.
There was a problem hiding this comment.
That's addressed in the first check, on line 69. For those skipped items, we don't have a temp log either. So the first check catches that and exits gracefully.
| IEnumerable<string>? fileContents = null; | ||
| Stopwatch fileReadStopwatch = Stopwatch.StartNew(); | ||
|
|
||
| while (fileReadStopwatch.Elapsed.Seconds < 60) |
There was a problem hiding this comment.
Nit - while mostly functional in practice, technically this is incorrect as the Elapsed property returns a TimeSpan representing the elapsed time and only checking its Seconds field would for instance yield zero at a one minute mark, I believe this is the purpose of the ElapsedTicks and ElapsedMilliseconds properties on Stopwatch.
There was a problem hiding this comment.
Hmm, in fact, upon second reading I think it's completely incorrect as the Seconds property is always in the range 0-59 so this is basically an infinite loop in case the file remains locked forever.
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Thanks @ivdiazsa for the fix. |
ghost
locked as resolved and limited conversation to collaborators
Addresses #83298.