-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add timestamps to XUnitLogChecker output #86737
Conversation
Tagging subscribers to this area: @hoyosjs Issue Detailsnull
|
Failures are marked known plus #85993 (new run- no failures) |
@ivdiazsa Does adding these timestamps seem reasonable to you? |
return SUCCESS; | ||
} | ||
|
||
static void WriteLineTimestamp(string message) | ||
{ | ||
Console.Write($"[XUnitLogChecker]: {System.DateTime.Now:HH:mm:ss.ff}: "); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is any concurrent console output possible? I assume not but if it is you might want to make this a single write to the console not two.
/azp run runtime-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think adding the timestamps will definitely prove to be a helpful change. Thanks for doing this work, Mark! And thanks for keeping the format to avoid super long lines of code. LGTM!
I just kicked off an outerloop CI run to be safe. I've had experiences of those breaking in the most unexpected ways 😅. If all's good there, then feel free to merge this unless someone has a reason not to do it yet.
Failures are unrelated to this change, so merging now. |
Thank you for reviewing, launching the test, and merging! |
The main motivation here is to give a quick notification whether a timeout was due to taking a long time or coincidentally hitting some external limit at that point. I think the below is due to the
watchdog 119
hitting, and since 9:31:47 isn't long after 9:31:44, then the thread07 test is probably stuck, but I'm never sure that I'm reading it correctly.Here's a snippet from a successful test run for this PR. If the "passed test" line was missing, we would still easily be able to bound the test time under 3 seconds.