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

Failing Tests - GivenIWantToParseImplicitCurrency #79

Open
Allann opened this issue Dec 11, 2019 · 2 comments
Open

Failing Tests - GivenIWantToParseImplicitCurrency #79

Allann opened this issue Dec 11, 2019 · 2 comments
Labels
Milestone

Comments

@Allann
Copy link

Allann commented Dec 11, 2019

I have been using the library and have found that I get errors randomly on first use.

I believe I have tracked this down to a race condition in the CurrencyRegistry when constructing the list of currencies. If the currency is not returned it uses the CurrentCulture's currency, which in my case is AUD, but usually using USD in this app.

If you run all tests you'll find that random parse tests fail, this is a symptom of the race problem.

Example test output:

NodaMoney.Tests.MoneyParsableSpec.GivenIWantToTryParseImplicitCurrency.WhenParsingDollarSymbolInArgentina_ThenThisShouldReturnArgentinePeso
   Source: MoneyParsableSpec.cs line 363
   Duration: 3 ms

  Message: 
    NodaMoney.InvalidCurrencyException : The requested operation expected the currency AUD, but the actual value was the currency ARS!
  Stack Trace: 
    Money.AssertIsSameCurrency(Money left, Money right) line 297
    Money.CompareTo(Money other) line 122
    ComparableTypeAssertions`1.Be(T expected, String because, Object[] becauseArgs)
    GivenIWantToTryParseImplicitCurrency.WhenParsingDollarSymbolInArgentina_ThenThisShouldReturnArgentinePeso() line 368
@ptjhuang
Copy link

Set xunit MaxParallelThreads to 1 to allow correct thread culture reset between tests.

Not hearing much response from the maintainers, so the fix has been implemented on our own fork.

RemyDuijkeren pushed a commit that referenced this issue Mar 29, 2020
- merged NodaMoney.Serialization.AspNet with NodaMoney
- improved ISO4127 testing and enabled them
- Fix #79
- removed .NET 4.0 and .NET 4.5 support
@RemyDuijkeren
Copy link
Owner

Fixed by using [Collection(nameof(NoParallelization))] on test that can break under high load of culture switching.

@RemyDuijkeren RemyDuijkeren added this to the v2.0 milestone Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants