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

Flaky test: UnitsNet.Tests.UnitSystemTests.PositiveInfinityFormatting #436

Closed
angularsen opened this issue May 5, 2018 · 3 comments
Closed

Comments

@angularsen
Copy link
Owner

This only happens very rarely on AppVeyor (first time I can think of), but @gojanpaolo reports it happens all the time on his setup. I assume this is due to manipulating static values in other tests, such as:

GetDefaultAbbreviationFallsBackToUsEnglishCulture()
{
    var zuluCulture = new CultureInfo("zu-ZA");
    CultureInfo.CurrentCulture = CultureInfo.CurrentUICulture = zuluCulture;

https://ci.appveyor.com/project/angularsen/unitsnet/build/724/tests

UnitsNet.Tests.UnitSystemTests.PositiveInfinityFormatting

Assert.Equal() Failure
          ↓ (pos 0)
Expected: ∞ m
Actual:   Infinity m
          ↑ (pos 0)

The following:

Console.WriteLine(double.PositiveInfinity.ToString(CultureInfo.InvariantCulture));
Console.WriteLine(double.PositiveInfinity.ToString(new CultureInfo("zu-ZA")));

/* Prints:
Infinity
∞
*/

So the fix is likely one or both of:

  1. Ensure tests that rely on manipulating CultureInfo or other static values are NOT run in parallel (can xunit selectively configure this?)
  2. Set CultureInfo properties back to their original values so it doesn't pollute other tests in same test class (I think all tests in one class are run in the same app domain and share static values)
@gojanpaolo
Copy link
Contributor

I think fix#1 is possible by applying test collection definition

I'll try fix#2 also then submit a PR whichever fixes the issue.

@gojanpaolo
Copy link
Contributor

gojanpaolo commented May 5, 2018

It seems like my system's default InfinitySymbol is "Infinity" instead of "∞" thus the failing test.

I tried just running the InfinityFormatting test only and still fails.
I also created a new project and double.NegativeInfinity.ToString() still returns "-Infinity".

We can pursue implementing the two fixes suggested earlier though I'm not sure how we can verify that it does resolve the issue.

@angularsen
Copy link
Owner Author

It would be hard to verify.

You are also right that on computers with a different culture, this test may always fail since it doesn't try to specify a culture. That test should specify InvariantCulture, which is a third fix on this problem - but it only fixes this particular test.

I would do all 3 really, since they all seem like the right thing to do in my mind. Then I also wouldn't care too much to try to verify if it actually fixes it, let's just assume it does until it proves us wrong down the road ;)

angularsen added a commit that referenced this issue May 8, 2018
…ormatting (#438)

* Fix #436 Flaky test: UnitsNet.Tests.UnitSystemTests.PositiveInfinityFormatting

Fixes #436

Added `UnitSystemFixture` class which is a `CollectionDefinition` class used to group test classes:
1. that rely on manipulating CultureInfo. See #436
2. to avoid accessing static prop DefaultToString in parallel from multiple tests:
	a. UnitSystemTests.DefaultToStringFormatting()
	b. LengthTests.ToStringReturnsCorrectNumberAndUnitWithCentimeterAsDefualtUnit()

Changes in this PR:
- `UnitSystemTests.GetDefaultAbbreviationFallsBackToUsEnglishCulture`: Set CultureInfo properties back to their original values.
- Specify `InvariantCulture` on the following `UnitSystemTests` tests:
	1. `NegativeInfinityFormatting`
	2. `PositiveInfinityFormatting`
- Applied `[Collection(nameof(UnitSystemFixture)]` attribute to test classes that:
	- Sets `UnitSystem.DefaultCulture`
	- Originally uses `[Collection("DefaulToString")]` that also depends on `CultureInfo`
		- Only one `Collection` attribute per test class.
- Updated to `xunit` and `xunit.runner.visualstudio` to v2.3.1 (was v2.3.0-beta4-build3742)
	- To have the [CollectionDefinition(DisableParallelization = true)] feature. See https://xunit.github.io/releases/2.3-beta5

* Removed `InvariantCulture`

Removed `InvariantCulture` on the following `UnitSystemTests` tests:
	1. `NegativeInfinityFormatting`
	2. `PositiveInfinityFormatting`

* Specified InvariantCulture in InfinityFormatting Tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants