Skip to content

Fix Idn failing tests #115018

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

Merged
merged 2 commits into from
Apr 25, 2025
Merged

Fix Idn failing tests #115018

merged 2 commits into from
Apr 25, 2025

Conversation

tarekgh
Copy link
Member

@tarekgh tarekgh commented Apr 24, 2025

Fixes #115006

@Copilot Copilot AI review requested due to automatic review settings April 24, 2025 22:05
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses failing tests by updating the ICU version threshold for selecting the appropriate IdnaTest file.

  • Lower the version threshold for selecting "IdnaTest_15_1.txt" to 72.
  • Adjust the test configuration in the Factory.cs file to match expected ICU behavior.

Copy link
Contributor

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

Copy link
Contributor

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are also failing in the prev4 branch. Can you please backport? It would be tell-mode, I can merge it.

Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if it fixes the issue. Seems like we're updating based on observation of which ICU versions we see in the places we test - anything we need to do to look at the ICU build / repo to find exact version where this change was introduced?

@carlossanlop
Copy link
Contributor

Confirmed it's also affecting prev4: https://github.com/dotnet/runtime/pull/114979/checks?check_run_id=41117511379

@ericstj
Copy link
Member

ericstj commented Apr 24, 2025

Confirmed it's also affecting prev4: https://github.com/dotnet/runtime/pull/114979/checks?check_run_id=41117511379

Yes, I opened the issue based on a P4 PR

@tarekgh
Copy link
Member Author

tarekgh commented Apr 24, 2025

/backport to release/10.0-preview4

Copy link
Contributor

Started backporting to release/10.0-preview4: https://github.com/dotnet/runtime/actions/runs/14652881804

@tarekgh
Copy link
Member Author

tarekgh commented Apr 24, 2025

/backport to release/9.0-staging

Copy link
Contributor

Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/14653792031

Copy link
Contributor

@tarekgh backporting to "release/9.0-staging" failed, the patch most likely resulted in conflicts:

$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Fix Idn failing tests
Using index info to reconstruct a base tree...
M	src/libraries/System.Runtime/tests/System.Globalization.Extensions.Tests/IdnMapping/Data/Factory.cs
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Runtime/tests/System.Globalization.Extensions.Tests/IdnMapping/Data/Factory.cs
CONFLICT (content): Merge conflict in src/libraries/System.Runtime/tests/System.Globalization.Extensions.Tests/IdnMapping/Data/Factory.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix Idn failing tests
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

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

Successfully merging this pull request may close these issues.

System.Globalization.Tests.IdnMappingIdnaConformanceTests failing on Windows
3 participants