Skip to content
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

[browser][HybridGlobalization] Improve logging on FirstDayOfWeek failures #93380

Merged
merged 4 commits into from
Oct 16, 2023

Conversation

ilonatommy
Copy link
Member

I cannot reproduce the failure from CI. We are testing hundreds of locales in the same test and logging only:

info: Expected: Monday
info: Actual:   Sunday

on failure. Xunit's Assert.Equal does not have a message argument overload, so it was changed to Assert.True that has it. Now we will get e.g. Failed for culture: pl-PL. Expected: Monday, Actual: Sunday.
In order to change only hybrid-focused test cases the non-hybrid cases were moved to a separate test.

@ghost
Copy link

ghost commented Oct 12, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

I cannot reproduce the failure from CI. We are testing hundreds of locales in the same test and logging only:

info: Expected: Monday
info: Actual:   Sunday

on failure. Xunit's Assert.Equal does not have a message argument overload, so it was changed to Assert.True that has it. Now we will get e.g. Failed for culture: pl-PL. Expected: Monday, Actual: Sunday.
In order to change only hybrid-focused test cases the non-hybrid cases were moved to a separate test.

Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-System.Globalization

Milestone: -

We do not test Invariant and en-US in hybrid mode, so it's fine to run it there as well.
@ilonatommy
Copy link
Member Author

It turns out the failure is not on HybridGlobalization-specific test but on the general one. However, this fix will be helpful for the future anyway.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants