Skip to content

Enable localization testing since all resources are translated#13162

Merged
LeafShi1 merged 2 commits intodotnet:mainfrom
LeafShi1:Issue_13117_Enable_Localization_tests
Mar 24, 2025
Merged

Enable localization testing since all resources are translated#13162
LeafShi1 merged 2 commits intodotnet:mainfrom
LeafShi1:Issue_13117_Enable_Localization_tests

Conversation

@LeafShi1
Copy link
Member

@LeafShi1 LeafShi1 commented Mar 19, 2025

Fixes #13117

Proposed changes

  • Enable following localization tests since all resources are translated
    • System.Windows.Forms.Tests.KeysConverterTests.ConvertFrom_ShouldConvertKeys_Localization
    • System.Windows.Forms.Tests.ToolStripMenuItemTests.ToolStripMenuItem_SetShortcutKeys_ReturnExpectedShortcutText
    • System.Windows.Forms.Tests.KeysConverterTests.ConvertToString_ShouldConvertKeys_Localization
  • Correct the translation for (none) in French from (aucune) to (aucun)
Microsoft Reviewers: Open in CodeFlow

@LeafShi1 LeafShi1 requested a review from a team as a code owner March 19, 2025 02:54
<trans-unit id="toStringNone">
<source>(none)</source>
<target state="translated">(aucune)</target>
<target state="translated">(aucun)</target>
Copy link
Member Author

Choose a reason for hiding this comment

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

The test case ConvertFrom_ShouldConvertKeys_Localization failed because the (none) was translated to (aucune), this should be a translation error.
In the VS repo and previous translations, none here is translated into aucun.

<trans-unit id="toStringNone">
<source>(none)</source>
<target state="translated">(aucune)</target>

Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik Mar 19, 2025

Choose a reason for hiding this comment

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

@cristianosuzuki77 - could you please double-check this translation? The new version "aucune" is used when speaking about humans, and the previous was for speaking about animals or objects.

Copy link
Contributor

Choose a reason for hiding this comment

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

@LeafShi1 - is it possible to use the SR resource name to retrieve the localized string in the test.

Copy link
Member Author

@LeafShi1 LeafShi1 Mar 20, 2025

Choose a reason for hiding this comment

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

Do you mean like this?

// Using the SR resource name to retrieve the localized string.
 var localizedString = SR.toStringNone;

 CultureInfo culture = CultureInfo.GetCultureInfo("fr-FR");
 KeysConverter converter = new();
 var resultFromSpecificCulture = (Keys?)converter.ConvertFrom(context: null, culture, localizedString);

 Assert.Equal(Keys.None, resultFromSpecificCulture);

But it will failed in non-english OS when executing (Keys?)converter.ConvertFrom(context: null, culture, localizedString);
Like the exception in CHS OS: System.ArgumentException : Requested value '(无)' was not found.,
(The language to be verified is French, but the resource value obtained through the source name is Chinese characters
In this case, we have to convert the localized string into a unified format before executing converter.ConvertFrom,this makes the test more complicated)

Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik Mar 24, 2025

Choose a reason for hiding this comment

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

@LeafShi1 - we are not supposed to edit the .xlf files. These files are generated from Cristiano's database. This is why we got this string changed back to aucune https://github.com/dotnet/winforms/pull/13176/files. To change a translation, we open bugs in aka.ms/icxlocbug.

Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik Mar 24, 2025

Choose a reason for hiding this comment

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

I think we have methods (GetString??) on the SR class that pass in the desired culture, we can't use var localizedString = SR.toStringNone;

Copy link
Member Author

Choose a reason for hiding this comment

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

@LeafShi1 - we are not supposed to edit the .xlf files. These files are generated from Cristiano's database. This is why we got this string changed back to aucune https://github.com/dotnet/winforms/pull/13176/files. To change a translation, we open bugs in aka.ms/icxlocbug.

OK, got it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we have methods (GetString??) on the SR class that pass in the desired culture, we can't use var localizedString = SR.toStringNone;

I created #13192: use the SR resource name to retrieve the localized string, please review it.

@codecov
Copy link

codecov bot commented Mar 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.35829%. Comparing base (e0ac569) to head (365b628).
Report is 6 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #13162         +/-   ##
===================================================
+ Coverage   61.35450%   61.35829%   +0.00378%     
===================================================
  Files           1547        1547                 
  Lines         158479      158479                 
  Branches       14751       14751                 
===================================================
+ Hits           97234       97240          +6     
+ Misses         60543       60536          -7     
- Partials         702         703          +1     
Flag Coverage Δ
Debug 61.35829% <ø> (-35.38836%) ⬇️
integration 10.73185% <ø> (∅)
production 39.23119% <ø> (∅)
test 95.67947% <ø> (-1.06718%) ⬇️
unit 36.66262% <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@LeafShi1 LeafShi1 added the waiting-review This item is waiting on review by one or more members of team label Mar 19, 2025
@Tanya-Solyanik Tanya-Solyanik removed the waiting-review This item is waiting on review by one or more members of team label Mar 19, 2025
@LeafShi1 LeafShi1 merged commit 1f3ba53 into dotnet:main Mar 24, 2025
8 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0 Preview4 milestone Mar 24, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Apr 26, 2025
Sign up for free to 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.

[Tracking] Localization tests under System.Windows.Forms.Tests failed

2 participants