Skip to content

Fix loc failures in IDE#25797

Merged
jaredpar merged 4 commits intodotnet:features/spanish-queuefrom
jaredpar:fix-loc
Apr 1, 2018
Merged

Fix loc failures in IDE#25797
jaredpar merged 4 commits intodotnet:features/spanish-queuefrom
jaredpar:fix-loc

Conversation

@jaredpar
Copy link
Copy Markdown
Member

No description provided.

The tests are using strings from libraries whose loc we don't control.
Hence only running these in english.

closes dotnet#25764
@jaredpar jaredpar added the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Mar 28, 2018
@jaredpar jaredpar requested a review from a team as a code owner March 28, 2018 20:35
}

[Fact, Trait(Traits.Feature, Traits.Features.Workspace)]
[ConditionalFact(typeof(IsEnglishLocal)), Trait(Traits.Feature, Traits.Features.Workspace)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't this be called IsEnglishLocale?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes it probably should.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I also have a question about this. As I understand it, this will cause the tests to be skipped on a non-english culture, which doesn't seem like an ideal solution. The test should instead somehow enforce the culture to be en-US so that it runs on any machine.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I agree in principal here. Right now though our primary goal is to get the Spanish Jenkins leg green and merged back into our production branches. This is important to

  1. Ensure our non-English contributors are unblocked.
  2. Ensure that we don't further regress any localization scenarios.

I'm trying to fix any tests that have obvious quick fixes. For others though I'm switching them to English only and leaving our localization bugs active to ensure we follow up on them.

We do have a mechanism for forcing the suite to run under English but it doesn't appear to be functioning correctly at the moment. That's why I'm using conditional execution on these items for now.

@jaredpar jaredpar requested a review from a team as a code owner March 30, 2018 14:57
End Function

<WpfFact>
<ConditionalWpfFact(GetType(IsEnglishLocal))>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you mention the tracking bug?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done

{
double result;
if (!double.TryParse(data[j], out result))
if (!double.TryParse(data[j], NumberStyles.Float | NumberStyles.AllowThousands, EnsureEnglishUICulture.PreferredOrNull, out result))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@heejaechang @sharwell @ivanbasov Are the files guaranteed to be in the English locale during generation?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Know the IDE team typically prefers current culture over the compiler preference of forcing English. In this case though the CSV files are checked into the code base and exposed as an embedded resource. Hence we need to pick a fixed culture to use during parse here as they copy will always be in a single locale.

@jaredpar
Copy link
Copy Markdown
Member Author

test windows_debug_spanish_unit32_prtest please

@jaredpar jaredpar removed the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Mar 30, 2018
@jaredpar
Copy link
Copy Markdown
Member Author

@dotnet/roslyn-compiler please review. Really small test change.

@jcouv jcouv added Test Test failures in roslyn-CI Area-IDE Area-Compilers labels Mar 30, 2018
Copy link
Copy Markdown
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (iteration 4)

@jaredpar jaredpar merged commit fc2a9a1 into dotnet:features/spanish-queue Apr 1, 2018
@jaredpar jaredpar deleted the fix-loc branch April 1, 2018 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Area-IDE Test Test failures in roslyn-CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants