Re enable tests for operators: OperatorsModule1.fs and OperatorsModule2.fs

[abelbraaksma]

Fixes #9515.

While looking for tests for string (which needs a fix, see #7958 and #9153) I found them, but they were not enabled. The PR re-enables those tests and a bunch of tests for other top-level operators like sin, float etc. I believe this has merit (in fact, it showed an unexpected behavior, see failing test for tanh in 2nd commit).

These tests were very old, and sometimes used syntax that is not valid anymore, or functions that have changed signatures (yes, it happens). I brushed them up a bit. Apart from tanh, they all succeed now.

This changes the project file (first commit), which may require a bit of scrutiny.

[abelbraaksma]

I've reported the seemingly "wrong" tanh test, which shouldn't be wrong, here, with explanation of the cause: #9522

I've reported the issue with using Assert.IsTrue using a boolean from an expression like x = 0.5 here: #9517

[abelbraaksma]

Cleaned it up a bit. This can be reviewed, tests should be green now. I also re-enabled some long-gone tests, which for safety reasons ought to be enabled, I think.

cc @vzarytovskii, including you in here (I don't know how to request a review to you specifically, but since you're redesigning the tests, I figured you ought to be included here. I tried to minimize the changes for NUnit, given you're moving to xUnit).

[abelbraaksma]

Assertion: Expected 1.266103673 but got 1.266103673

While I re-enabled the messages (because exception was originally used, and F#'s exception type hides the Message property by shadowing it instead of overriding -- old bug), we now indeed get messages but still not very useful, because the standard float-to-string conversion is not very specific.

I'll try to fix that too. If anything, this means that this specific build runs in x64, because I tested the build from within VS, which runs in x86. Wondering if both builds are tested?

[abelbraaksma]

I can't seem to repro the last errors locally, either using 64-bit or 80-bit arithmetic (acos etc failing), so I've updated the test-output for cases of failure, where specially for float and float32 a higher precision is shown in the logs. Hopefully this helps pinpoint the issue with the failing tests.

[abelbraaksma]

Assertion: Expected 1.2661036727794992 but got 1.266103672779499

At least now the error makes sense. @KevinRansom / @cartermp, is there a way I can see what version of CLR and 32 vs 64 is running? I can also add a meta test that dumps this info on any run, but where would that show, is there a way I can access the raw console output?

[abelbraaksma]

It gets weirder (and perhaps this is a bug in F#, not sure). Consider:

    [<Test>]
    member this.ByteOverflow() =
        // Overflow
        let result = Operators.byte (Int64.MaxValue * 8L)
        Assert.AreEqual(248uy, result)      // bit-complement

        // Overflow
        let result = 255uy + 5uy
        Assert.AreEqual(4uy, result)

The code above does not use Checked and so should not raise an OverflowException. When run with NUnit, NCrunch, FSI, dotnet FSI, after compile, x86 FSI and x64 FSI: nothing raises this (good!). Except when run in MSTest window in VS opened from FSharp.sln: it fails on the first Operators.byte line with an OE.

This is in the latest commit. Wonder what CI says of this. But be as it may, this should never raise, so I'm just hoping that the VS Test window is confused or something... Can anyone confirm this indeed work/does not work in VS? (@KevinRansom, @cartermp)?

Is there any configuration weirdness I should be aware of here? Also of note: other overflow-checks run fine in VS Test window (they don't throw, unless Checked is used).

[abelbraaksma]

Ah, just found the discrepancy, those files have:

open Microsoft.FSharp.Core.Operators.Checked

So any arithmetic operator is checked. I'll separate this. I wonder if that's also causing the other oddities.

[abelbraaksma]

Latest change has one new peculiarity, as locally I noticed a difference in outcome of calculations whether or not netcore was running the tests or not. There may still be failing tests because I haven't yet figured out how to ascertain whether the CI runners use an x86 or x64 environment, or both.

Newfound oddity is logged: #9526 (should be fixed in the tests now)

/cc @cartermp, @KevinRansom, I need some help about what runtimes are used for the CI builds & test-runs.

[abelbraaksma]

This is a lost war. I have no idea what runtimes are used on the various CI builds, so I won't be able to verify whatever goes wrong locally. Since all issues are floating point related, I'm going to introduce AreNearEqual which makes sure fp values are equal within a certain delta.

Atm I've set the delta to 1e-15, which seems reasonable. Locally it works, but has locally been working for a day already so that doesn't say much.

[abelbraaksma]

Darn, sourcelink error again. This PR is not going down easy 😁

##[error]C:\Users\VssAdministrator\.nuget\packages\microsoft.sourcelink.common\1.1.0-beta-20206-02\build\InitializeSourceControlInformation.targets(3,81): error MSB4022: The result "" of evaluating the value "$(_MicrosoftSourceLinkCommonAssemblyFile)" of the "AssemblyFile" attribute in element <UsingTask> is not valid.
C:\Users\VssAdministrator\.nuget\packages\microsoft.sourcelink.common\1.1.0-beta-20206-02\build\InitializeSourceControlInformation.targets(3,81): error MSB4022: The result "" of evaluating the value "$(_MicrosoftSourceLinkCommonAssemblyFile)" of the "AssemblyFile" attribute in element <UsingTask> is not valid. [D:\a\1\s\src\fsharp\FSharp.DependencyManager.Nuget\FSharp.DependencyManager.Nuget.fsproj]
##[error]C:\Users\VssAdministrator\.nuget\packages\microsoft.sourcelink.common\1.1.0-beta-20206-02\build\InitializeSourceControlInformation.targets(3,81): error MSB4022: The result "" of evaluating the value "$(_MicrosoftSourceLinkCommonAssemblyFile)" of the "AssemblyFile" attribute in element <UsingTask> is not valid.

[abelbraaksma]

@KevinRansom, @cartermp, except from the sourcelink errors, it is now green. I committed a few new tests to trigger the build again, see if it's now all green: EDIT, all green, yay!

It's ready for review.

[KevinRansom]

Minor question, otherwise this looks great.

Thanks

Kevin

[cartermp]

This is great work @abelbraaksma

Will want @vzarytovskii to take a look here

[vzarytovskii]

Looks good to me. Thanks.