Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

@steveharter
Copy link
Contributor

No description provided.

@stephentoub
Copy link
Member

@dotnet-bot test this please

@stephentoub
Copy link
Member

@dotnet-bot test this please (after Jenkins changes related to setting culture-related env vars)

@steveharter
Copy link
Contributor Author

@dotnet-bot test this please

@stephentoub
Copy link
Member

@steveharter, given what we saw today with Jenkins and the various LANG/LC_*/etc. env vars, are there any additional tests we can/should add to validate behavior in such cases in the future?

@steveharter
Copy link
Contributor Author

@stephentoub the other tests that were failing should probably be using Invariant culture explicitly and not assume en-us or other compatible culture for string formatting... For now, however, we could verify en-us in this test.

@steveharter
Copy link
Contributor Author

FWIW, the default locale if no LANG is set is to use en-US-POSIX which has different default formatting settings than en-US or invariant.

@steveharter
Copy link
Contributor Author

Four potential ways to address:

  1. Do nothing
  2. Change the tests that were failing to explicitly use Invariant
  3. Add a test to verify en-US for Linux as the default locale
  4. Change functionality to detect en-US-POSIX from Linux and then return the Invariant locale's culturedata or a copy of it (not sure if this is a valid approach)

@steveharter
Copy link
Contributor Author

@ellismg do you have thoughts on the best way to address. I prefer option 2 (change tests to use invariant) and\or option 3 (verify en-us in unit test). Thanks

@steveharter
Copy link
Contributor Author

FYI the list of tests that assume en-us or invariant (UPDATE: some of these tests fail due to unrelated case-insensitive string comparison failing for en-US-POSIX)
Int32Tests.TestTryParse
UInt32Tests.TestTryParse
UInt16Tests.TestTryParse
UInt64Tests.TestTryParse
StringTests.TestLastIndexOf
StringTests.TestEndsWith
StringTests.TestStartsWith
StringTests.TestIndexOf
Int16Tests.TestTryParse
Int64Tests.TestTryParse
TypeTests.TestGetTypeByName
Tests.Expression_Tests.TestCallInstanceMethodsByName
CaseInsensitiveComparer_ctor.ExecuteCaseInsensitiveComparer_ctor
CaseInsensitiveComparer_Default.ExecuteCaseInsensitiveComparer_Default
CaseInsensitiveComparer_Object.ExecuteCaseInsensitiveComparer_Object
CollectionsUtilTests.ExecuteCollectionsUtilTests
System.Reflection.Tests.ModuleTest.GetTypeTest
System.Reflection.Tests.MethodInfoPropertyTests.TestReturnType2
System.Reflection.Tests.MethodInfoPropertyTests.TestReturnType4
System.Reflection.Tests.GetTypeReflectionTests.GetType1
System.Reflection.Tests.ParameterInfoNameTests.TestMethodParams1

@ellismg
Copy link
Contributor

ellismg commented Oct 13, 2015

I would prefer if these tests use Invariant.

@stephentoub
Copy link
Member

Should we merge these test changes, and then address the non-invariant issue separately?

@steveharter
Copy link
Contributor Author

Added issue #3850 to address the tests listed above. I will merge this pull request once I get the OK

Copy link
Member

Choose a reason for hiding this comment

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

This is pre-existing, but we should probably be resetting the current culture in a finally block. That way, if there's a bug that causes one of the asserts to fail, subsequent tests won't be thrown off by the change.

@stephentoub
Copy link
Member

One pre-existing nit, otherwise LGTM.

stephentoub added a commit that referenced this pull request Oct 14, 2015
Add test for non invariant default locale and collation
@stephentoub stephentoub merged commit 839706d into dotnet:master Oct 14, 2015
@karelz karelz modified the milestone: 1.0.0-rtm Dec 3, 2016
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…leTest

Add test for non invariant default locale and collation

Commit migrated from dotnet/corefx@839706d
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants