SCAN4NET-96 and SCAN4NET-97: Fix logger timestamp duplication #2212
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gregory-paidis-sonarsource
force-pushed
the
greg/scan4net-96-and-97
branch
from
417670e to
a5c35f6
Compare
| public void AssertExpectedLastMessageRegex(string expected) | ||
| { | ||
| var lastMessage = GetLastMessage(outputWriter); | ||
| lastMessage.Should().MatchRegex(expected, "Expected message was not logged"); |
There was a problem hiding this comment.
Need to match the timestamp abstractly, as it does not have a concrete, static value.
| @@ -111,7 +111,7 @@ public void ResumeOutput() | |||
| } | |||
|
|
|||
| public void LogWarning(string message, params object[] args) => | |||
| Write(MessageType.Warning, GetFormattedMessage(Resources.Logger_WarningPrefix + message, args)); | |||
| Write(MessageType.Warning, Resources.Logger_WarningPrefix + message, args); | |||
There was a problem hiding this comment.
There were three bugs with this file:
LogWarningANDLogUIWarningused to callGetFormattedMessage. Every invocation adds one more timestamp, and since theUImethod calls the normal warning one, we would get 2 of them.Writewould add one moreGetFormattedMessageon the top, so we would get 3 of them.FlushOutputthen would also callGetFormattedMessage, so we would get 4 of them.
My refactoring does the following two main steps:
- The only place where
GetFormattedMessageis allowed to be called is insideWrite. This means everyLogmethod falls-through toWrite, where the formatting happens. - The
FlushOutputdoes not callWriteat all, but a method that is wrapped byWrite, which already expects the message to be formatted.
|
gregory-paidis-sonarsource
requested a review
from costin-zaharia-sonarsource
costin-zaharia-sonarsource
approved these changes
Sep 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SCAN4NET-96
SCAN4NET-97