Shared.Tests - Add Tests

[JoseBritto]

Tests added for CurrencyHelpers to make sure it works with all cultures.

I hardcoded some values because in the tests we know exactly what values go into the test methods and what the expected output is.

Currently all tests are passing except "kea-CV" and "pt-CV". These cultures have $ in the middle of the number:
$1.00 -> 1$00
$2.05 -> 2$05
Also for some reason they also have a zero width space at the end of the number.

[fsobolev]

Currently all tests are passing except "kea-CV" and "pt-CV". These cultures have $ in the middle of the number:
$1.00 -> 1$00
$2.05 -> 2$05
Also for some reason they also have a zero width space at the end of the number.

The values seem to be correct: these cultures don't have currency sign (that probably explains space at the end) and $ as decimal separator.
kea-CV pt-CV

[JoseBritto]

Those are the expected values. Currently those tests fail.

[fsobolev]

Those are the expected values. Currently those tests fail.

Ah okay, got it. Let's see...

[JoseBritto]

Assert.Equal() Failure
↓ (pos 0)
Expected: 0$00 ​
Actual: $00

Wasn't sure what the best way to solve it is, so I didnt touch the method itself

[JoseBritto]

TODO:

1. Fix ToAmountString to account for $ instead of decimal point
2. Add dotnet test to github workflows? (so we can have future tests automatically run with the other checks)

Not sure of the best way to do either of them at the moment.

[nlogozzo]

Add dotnet test to github workflows? (so we can have future tests automatically run with the other checks)

I'll make the workflow for this

[nlogozzo]

https://github.com/NickvisionApps/Denaro/actions/runs/6158369998

[JoseBritto]

Are we fixing the method separately? This one just makes the test pass but doesn't actually fix the problem 🤔 (about the tests)

[JoseBritto]

#708 (review)
I meant to leave that comment here. AAh that did the same thing 🤦
Meant to comment on Fyodor's commits

[fsobolev]

@JoseBritto let me sumarize info about problematic cultures to ensure I don't miss anything:

1. CurrencySymbol - \u200b (non-printable)
2. CurrencyDecimalSeparator - $
3. Format can't be C, it should be minimum C2 (unless overwriteDecimal is true), so 1.0 becomes 1$00
4. Currency symbol seem to cause misbehavior of string.Remove, string.Replace should be used instead

If all this is correct, I have a fix for the method itself, but the tests are complaining about symbol in the end (I see from errors reports that other than that the values are correct)

[JoseBritto]

@JoseBritto let me sumarize info about problematic cultures to ensure I don't miss anything:

1. `CurrencySymbol` - `\u200b` (non-printable)

2. `CurrencyDecimalSeparator` - `$`

3. Format can't be C, it should be minimum C2 (unless `overwriteDecimal` is true), so 1.0 becomes 1$00

4. Currency symbol seem to cause misbehavior of `string.Remove`, `string.Replace` should be used instead

If all this is correct, I have a fix for the method itself, but the tests are complaining about symbol in the end (I see from errors reports that other than that the values are correct)

Yes. I didn't actually care to check what the currency symbol was actually returning, assumed it was $ as that is what google told me and got confused when I got the invisible character at the end 🤦

[fsobolev]

3. Format can't be C, it should be minimum C2 (unless overwriteDecimal is true), so 1.0 becomes 1$00

Just noticed 1.2 was becoming 1$2 in AllCulturesShouldWorkWithOverwriteDecimal test, fixed actual value in latest commit.

[fsobolev]

Just one test left, this time it fails with mm-Mong-MN culture:

Assert.Equal() Failure
             ↓ (pos 3)
Expected: ₮10,920
Actual:   ₮10920
             ↑ (pos 3)

No idea why 🤔

[JoseBritto]

I managed to get all tests to pass and make the tests a lot simpler i think. Will pull in 4f7d2d0 and see if it works with that. If it does can i revert your latest commit? @fsobolev

[fsobolev]

Will pull in 4f7d2d0 and see if it works with that. If it does can i revert your latest commit? @fsobolev

Sure, feel free to do whatever you need :)

[JoseBritto]

Didn't have to revert. I'm bad at git lol. Anyway, now it should look better than the mess I made yesterday

[nlogozzo]

LGTM! Any remaining issues?

[JoseBritto]

Not that I know of. I'll wait for @fsobolev just in case

[fsobolev]

Ouch, I didn't know rebase adds person who make a rebase as co-author (at least when using Github Desktop), I hope nobody minds

[JoseBritto]

Ouch, I didn't know rebase adds person who make a rebase as co-author (at least when using Github Desktop), I hope nobody minds

I don't mind lol. This should now be good to merge right?

[fsobolev]

This should now be good to merge right?

Yes, I can't merge because I'm the last pusher

[nlogozzo]

LGTM

[anaximeno]

Thanks for implementing the "kea-CV" and "pt-CV", I was here to check something related to this, but seen you've already implemented it, so thanks again.