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

[wasm] Re-enable previously failing test - System.Globalization.Tests.RegionInfoPropertyTests.DisplayName #70711

Closed
wants to merge 1 commit into from

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Jun 14, 2022

No failures described in #45951 happen anymore. Runfo does not have any failures of System.Globalization.Tests.RegionInfoPropertyTests.DisplayName and running libs tests locally on Windows machine with 3 language packs no failures are recorded. Most probably got fixed by #30132. This is to remove the comment linking to the issue and making sure you agree with the decision.

Fixes #45951 .

@ghost
Copy link

ghost commented Jun 14, 2022

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

Issue Details

No failures described in #45951 happen anymore. Runfo does not have any failures of System.Globalization.Tests.RegionInfoPropertyTests.DisplayName and running libs tests locally on Windows machine with 3 language packs no failures are recorded. Most probably got fixed by #30132. This is to remove the comment linking to the issue and making sure you agree with the decision.

Author: ilonatommy
Assignees: -
Labels:

area-System.Globalization

Milestone: -

@radical radical changed the title Closing https://github.com/dotnet/runtime/issues/45951. [wasm] Re-enable previously failing test - System.Globalization.Tests.RegionInfoPropertyTests.DisplayName Jun 14, 2022
@radical radical added the arch-wasm WebAssembly architecture label Jun 14, 2022
@ghost
Copy link

ghost commented Jun 14, 2022

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

Issue Details

No failures described in #45951 happen anymore. Runfo does not have any failures of System.Globalization.Tests.RegionInfoPropertyTests.DisplayName and running libs tests locally on Windows machine with 3 language packs no failures are recorded. Most probably got fixed by #30132. This is to remove the comment linking to the issue and making sure you agree with the decision.

Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-System.Globalization

Milestone: -

@radical
Copy link
Member

radical commented Jun 14, 2022

/azp run runtime-libraries-mono outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical radical added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jun 14, 2022
@radical
Copy link
Member

radical commented Jun 14, 2022

In the last run of outerloop tests, this test was still failing. AFAICS, the outerloop pipeline is disabled from running automatically. And the last time it was run was this.

I'm running the pipeline here, so we can check the current state.

@tarekgh
Copy link
Member

tarekgh commented Jun 14, 2022

@radical I am seeing you enabled only Mono outer loop. Was the failure repro only with Mono? Looking at the original failure, it looks to me that when running with WASM without the globalization data the failure should occur.

@radical
Copy link
Member

radical commented Jun 14, 2022

@radical I am seeing you enabled only Mono outer loop.

I assume you meant, ran only the mono outerloop. The test was disabled only for Browser.
We can check the results from this run, and react accordingly.

@radical
Copy link
Member

radical commented Jun 14, 2022

The test still fails with:

[17:25:59] fail: [FAIL] System.Globalization.Tests.RegionInfoPropertyTests.DisplayName(name: "en-US", expected: "United States")
[17:25:59] info: Assert.Equal() Failure
[17:25:59] info:            ↓ (pos 1)
[17:25:59] info: Expected: United States
[17:25:59] info: Actual:   US
[17:25:59] info:            ↑ (pos 1)
[17:25:59] info:    at System.Globalization.Tests.RegionInfoPropertyTests.DisplayName(String name, String expected)
[17:25:59] info:    at System.Reflection.MethodInvoker.InterpretedInvoke(Object obj, Span`1 args, BindingFlags invokeAttr)

IIUC, we are building with InvariantGlobalization=false, so I would expect this to pass. I tried this with a regular build, and relinked.
This EnglishName test passes though.
@lewing @TanayParikh please correct me if I'm wrong.

So, if that's correct then we can close this PR, and the issue should remain open.

@ilonatommy
Copy link
Member Author

My bad, the test is indeed still failing. If anyone has anything against it, reopen the PR.

@ilonatommy ilonatommy closed this Jun 15, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Globalization NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[browser][globalization][tests] RegionInfoPropertyTests.DisplayName failures
4 participants