-
Notifications
You must be signed in to change notification settings - Fork 44
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
Always make available trace logs #557
Conversation
// Best effort clean-up, but we don't want to | ||
// fail the test because we couldn't delete this file | ||
} | ||
cancellationTokenSource.Cancel(); |
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.
No need to delete the log file.
but we do await the writing of logs, since we really want to know if writing logs doesn't work.
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.
Nice one, thanks! 👍🏻
} | ||
|
||
public bool CopyLogFileToArtifacts() | ||
public static DirectoryInfo LogFileDirectory() |
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.
Maybe this doesn't need to be public
anymore
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.
Ah probably
Logger = new SerilogLoggerBuilder() | ||
.SetTraceLogFileLogger(traceLogFileLogger) | ||
.Build() | ||
.ForContext(GetType()); | ||
|
||
Logger.Information("Trace log file {LogFile}", traceLogFileLogger.logFilePath); |
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.
Thanks for this bit, I've got some WIP code to associate the TeamCity artifact with the test run so the trace log file can be accessed in-line in the build log, but there's still some TeamCity black magic I need to figure out to get it to work.
This will help in the meantime!
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.
Ah nice!
I also like this message for local development, since when the test is run the first line is "here is more info if you need it"
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.
Yep, it was on my to-do list for that exact reason, but got distracted by the TeamCity black magic and it didn't make it to a PR.
Background
We always recorded trace logs to file, this PR changes the tests so that those trace logs are always available.
This means:
This test will also print the location of the trace log for easy debugging.
How to review this PR
Quality ✔️
Pre-requisites