Skip to content

Minor cleanup on Extensions in Configuration - #42788

Merged
maryamariyan merged 1 commit into
dotnet:masterfrom
maryamariyan:ready
Oct 2, 2020
Merged

Minor cleanup on Extensions in Configuration#42788
maryamariyan merged 1 commit into
dotnet:masterfrom
maryamariyan:ready

Conversation

@maryamariyan

@maryamariyanmaryamariyan commented Sep 27, 2020

Copy link
Copy Markdown
Contributor

Follow ups from older PRs. Checks off an item in #37821:

Using Invariant culture for remaining double params in EventSourceLoggerTest (#34208 (comment))

Related to: #39521 (removed commit ea24d1a)

cc: @tarekgh

Leaving EventSourceTest cleanup for later.

@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

@maryamariyanmaryamariyan changed the title Test fix up on Microsoft.Extensions.Logging.EventSource Test fix up on Microsoft.Extensions.Logging.EventSource for non-ASCII localesSep 27, 2020
@maryamariyanmaryamariyan changed the title Test fix up on Microsoft.Extensions.Logging.EventSource for non-ASCII localesMinor cleanup on Extensions in Logging.EventSource and ConfigurationSep 27, 2020
@tarekgh

Copy link
Copy Markdown
Member
 @"""ArgumentsJson"":{""timeParam"":""" + TimeParam.ToString() + @""",""guidParam"":""" + GuidParam.ToString("D")) },

does instances like this needed to be fixed tooo?


Refers to: src/libraries/Microsoft.Extensions.Logging.EventSource/tests/EventSourceLoggerTest.cs:957 in bbf2215. [](commit_id = bbf221590519ca2d4f3331397fe2cee294bc85b2, deletion_comment = False)

@maryamariyan

maryamariyan commented Sep 28, 2020

Copy link
Copy Markdown
ContributorAuthor

does instances like this needed to be fixed tooo?

I had and then reverted this commit ea24d1a from the PR (with this report: https://dev.azure.com/dnceng/public/_build/results?buildId=832928&view=ms.vss-test-web.build-test-results-tab&runId=26549100&resultId=102390&paneView=debug as it did not seem like a required change.)

e.g.:

Assert.Collection() Failure
\nCollection: ["{\"__EVENT_NAME\":\"MessageJson\",\"Level\":1,\"Fa"..., "{\"__EVENT_NAME\":\"MessageJson\",\"Level\":2,\"Fa"..., "{\"__EVENT_NAME\":\"ActivityJsonStart\",\"ID\":3,\"..., "{\"__EVENT_NAME\":\"MessageJson\",\"Level\":4,\"Fa"..., "{\"__EVENT_NAME\":\"MessageJson\",\"Level\":5,\"Fa"..., ...]\nError during comparison of item at index 5\nInner exception: Event data '{"__EVENT_NAME":"ActivityJsonStart","ID":4,"FactoryID":4,"LoggerName":"Logger3","ArgumentsJson":{"timeParam":"5/3/2016 7:00:00 PM","guidParam":"29bebd2c-7fa6-4e97-af68-b91fdaae24b6","{OriginalFormat}":"Inner scope {timeParam} {guidParam}"}}' does not contain expected fragment "ArgumentsJson":{"timeParam":"05/03/2016 19:00:00","guidParam":"29bebd2c-7fa6-4e97-af68-b91fdaae24b6
\n Expected: True\n Actual: False

 - used by two test projects
@maryamariyanmaryamariyan changed the title Minor cleanup on Extensions in Logging.EventSource and ConfigurationMinor cleanup on Extensions in ConfigurationOct 1, 2020
@maryamariyan
maryamariyan merged commit 67a39a4 into dotnet:masterOct 2, 2020
@ghostghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@maryamariyan@tarekgh