search

hedgehogqa / fsharp-hedgehog

Release with confidence, state-of-the-art property testing for .NET.
https://hedgehogqa.github.io/fsharp-hedgehog/
Other
271 stars 31 forks source link

Fix decimal inline conversion #441

Closed njlr closed 11 months ago

njlr commented 11 months ago

Closes #439

TysonMN commented 11 months ago

I removed the assert from Fable.

@ThisFunctionalTom, are you still interested in maintaining Fable support?

ThisFunctionalTom commented 11 months ago

I removed the assert from Fable.

@ThisFunctionalTom, are you still interested in maintaining Fable support?

I can look into it. Is there a problem with Fable? I could also update the compiler to the newest version. I'll try updating, compiling and running tests and see what comes out.

TysonMN commented 11 months ago

This is the build that I saw for the original version of this PR: https://github.com/hedgehogqa/fsharp-hedgehog/actions/runs/5660360646/job/15460970572

The diff was very confusing at first. Eventually I realized that there is a new line between each digit. So instead of a number like

1234

the diff is comparing

1
2
3
4

The expected values are the min and max values for decimal.

As I just worked on writing the above, now I see that the Fable build is also broken for master. Here is an example: https://github.com/hedgehogqa/fsharp-hedgehog/actions/runs/5705984309/job/15461213961

TysonMN commented 11 months ago

This is the build that I saw for the original version of this PR: https://github.com/hedgehogqa/fsharp-hedgehog/actions/runs/5660360646/job/15460970572

The diff was very confusing at first. Eventually I realized that there is a new line between each digit. So instead of a number like

1234

the diff is comparing

1
2
3
4

The expected values are the min and max values for decimal.

As I just worked on writing the above, now I see that the Fable build is also broken for master. Here is an example: https://github.com/hedgehogqa/fsharp-hedgehog/actions/runs/5705984309/job/15461213961

I just pushed a fix for that

TysonMN commented 11 months ago

@ThisFunctionalTom, thanks for PR #444.

Can this test assertion I excluded from Fable be included now?

If so, can you make a PR for that?

ThisFunctionalTom commented 11 months ago

@ThisFunctionalTom, thanks for PR #444.

Can this test assertion I excluded from Fable be included now?

If so, can you make a PR for that?

Sadly it still does not work.

I think I found a bug in fable compiler or Fable.Core libraries. Following test does not work:

    testCase "Fable Decimal/BigInt bug" <| fun _ ->
        let actual = Decimal.MinValue |> bigint |> decimal
        actual =! Decimal.MinValue

I am going to see if I can fix it myself or, if not, I will open an issue.