[Rust] How to set culture for unit tests

[dbrattli]

Description

Currently there's 11 tests that fails on machines with e.g Norwegian culture because results have commas instead of dots e.g:

Expected: %97.5
Actual:   %97,5

Not sure the best way to solve this. We can solve it using e.g CultureInfo.DefaultThreadCurrentCulture <- CultureInfo.InvariantCulture but this is not supported by Fable yet so we would need to #if FABLE_COMPILER or implement support for it. Another option is to use .ToString() to set culture info, but comes messy for string interpolation tests. I tried adding xunit.runner.json file to set culture for all tests but it seems to be ignored by the way tests are run. Suggestions?

[MangelMaxime]

Does the tests fails when running against .Net or rust runtime?

If this is only the first one, then we can probably use CultureInfo.DefaultThreadCurrentCulture <- CultureInfo.InvariantCulture

I don't remember exactly, but I think for JavaScript Fable compiles as if CultureInfo.InvariantCulture was always pass. <-- Needs confirmation because I am not sure at all.

[dbrattli]

Code fails when running .NET. We can fix it with CultureInfo.DefaultThreadCurrentCulture, but then we need to add support for it in Fable Rust as it gives error when compiling to Rust.

[ncave]

@dbrattli What is the error, can you post it or link to a build? Also, can we fix individual tests to use InvariantCulture?

[MangelMaxime]

Code fails when running .NET. We can fix it with CultureInfo.DefaultThreadCurrentCulture, but then we need to add support for it in Fable Rust as it gives error when compiling to Rust.

One solution I see, could be to wrap the line CultureInfo.DefaultThreadCurrentCulture with a compiler directives. But I don't know if this is just going to add stuff under the carpet or not ^^

[ncave]

@MangelMaxime @dbrattli Does adding this option to the test projects help?

  <PropertyGroup>
    <InvariantGlobalization>true</InvariantGlobalization>
  </PropertyGroup>

But perhaps we can just do what some other tests already do, and just replace the comma with a dot (for result validation only): https://github.com/fable-compiler/Fable/blob/main/tests/Js/Main/StringTests.fs#L252

[dbrattli]

Strangely the test passed at my Mac at home. Looking into things I noticed that the environment was different e.g LANG=en_US.UTF-8, so had to set it to Norwegian to get things failing again:

export LANG=nb_NO.UTF-8

Tried changing the project file as suggested above:

  <PropertyGroup>
    <InvariantGlobalization>true</InvariantGlobalization>
  </PropertyGroup>

This works! 🎉 so this is probably the simplest fix. I guess we can still override if we need to test culture changes for single tests later.

[dbrattli]

For reference, if you try to set culture within a single test e.g:

CultureInfo.CurrentCulture <- CultureInfo.InvariantCulture

Then the error you get is:

./../../../tests/Rust/tests/src/StringTests.fs(371,4): (371,104) error FABLE: System.Globalization.CultureInfo.set_CurrentCulture (static) is not supported by Fable
Compilation failed

[dbrattli]

But I think we anyways we should make setting CurrentCulture to InvariantCulture compilable for Rust. I think it already is for JS and Python, ref: https://github.com/fable-compiler/Fable/blob/main/src/Fable.Transforms/Python/Replacements.fs#L3085

UPDATE: failed for Python as well 👀

Can fix by allowing set_CurrentCulture to the same object expression that is returned by get_InvariantCulture. Does this look acceptable, or will it compile code that should have been failed?

let globalization (com: ICompiler) (ctx: Context) (_: SourceLocation option) t (i: CallInfo) (_: Expr option) (args: Expr list) =
    match i.CompiledName, args with
    | "get_InvariantCulture", _ ->
        // System.Globalization namespace is not supported by Fable. The value InvariantCulture will be compiled to an empty object literal
        ObjectExpr([], t, None) |> Some
    | "set_CurrentCulture", [ObjectExpr([], t, None)] ->
        ObjectExpr([], t, None) |> Some
    | _ -> None

[MangelMaxime]

@dbrattli Instead of generating "invalid code".

I think it is better to not support it and have the user use compiler directives:

#if !FABLE_COMPILER
CultureInfo.CurrentCulture <- CultureInfo.InvariantCulture
#endif

For me, this solution has the benefit of not introducing an edge case and just treat it as any non supported BCL API. What do you think?

[ncave]

@MangelMaxime I agree, even though it "compiles" on JS and Python, we don't actually support System.Globalization.CultureInfo, e.g. setting it to anything else besides InvariantCulture is not going to work as expected anyway.

We should check the culture value and provide a proper warning if not supported. We can add the CultureInfo type as an actual dummy type, for Rust and other typed languages. Arguably, it could be simpler to pass the dummy culture object to native implementations, instead of modifying the parameter list to remove it (as Rust currently does).