Fable tests

[ThisFunctionalTom]

Hi,

I just converted all of the Hedgehog tests from xunit to Fable.Mocha to test if everything is working OK.

There a 3 tests failing with Range.exponentialBounded. I put them in "pending" mode.
Otherwise it looks good.

Two tests are taking a lot of time "unicode doesn't return any surrogate" and "unicode doesn't return any noncharacter" because they generate 100000 values but they are green.
If you would like to speed the tests you can put them in pending state by changing the fact to pfact and theory to ptheory.

To run tests with nodjs use npm test
To run live tests in browser use npm start and then open http://localhost:8085

I also added expecto as the runner for Hedgehog.Fable.Tests. To run it you have to compile the Hedghog.Fable.Tests project with msbuild (dotnet build is not working for the reason you already know). After the build you can run Hedgehog.Fable.Tests like a normal console application and expecto will run the tests. There is also a npm run script npm run dotnet-expecto but you need to have msbuild in your path to use it.

I would also like to create some sample with Fable tests but I guess I will put it in another pull requests.

kind regards,
Tom

[moodmosaic]

Awesome 😍 I'll look into it during the next few days

[ThisFunctionalTom]

I also converted Hedgehog.Fable.Tests.fsproj to use paket and updated paket.exe to the newest version. Hope this is OK.

[inosik]

dotnet build is not working for the reason you already know

Do you run the latest VS 2019? The issue (failing do read FSharp.Core.dll) should be fixed in 16.5.3 and later.

[inosik]

SDK 3.1.201 was reported to be good again, but I can't build with it either.

[ThisFunctionalTom]

SDK 3.1.201 was reported to be good again, but I can't build with it either.

Yeah, I had the same problems.

Following is working for me:

In the root directory after fresh clone or after git clean -xdf

For running expecto tests:

dotnet restore
msbuild
dotnet run -p .\tests\Hedgehog.Fable.Tests\Hedgehog.Fable.Tests.fsproj

I am not sure why .paket\paket.exe restore is not working here but dotnet restore is working for me.

For running fable/mocha tests in nodejs (You have to wait a little bit, there are some long running tests)

cd .\tests\Hedgehog.Fable.Tests\
npm install
npm test

For running fable/mocha tests in browser (Here I have to click in browser to wait for the application because of the long running tests).
After npm start open http://localhost:8085 and wait.

cd .\tests\Hedgehog.Fable.Tests\
npm install
npm start

[moodmosaic]

This is so worth it 😲 🚀 Thank you, thank you, thank you, for all the great work. — If we'd use Expecto from start, now we would avoid all the duplication, right?

If that's the case, shouldn't we consider replacing the existing (xUnit.net) test-suite with just a single Expecto one?

As I see it, there's a huge benefit here: one test-suite for both native (F#) and compiled (JS headless and browser) targets.

[ThisFunctionalTom]

This is so worth it 😲 🚀 Thank you, thank you, thank you, for all the great work.

No problem. This is fun.

— If we'd use Expecto from start, now we would avoid all the duplication, right?

Yes. If we write everything in expecto style we can use it for Fable.Tests. The thing that does not work in Fable.Mocha is it does not support property based tests like Expecto but we are going to change it :-)

If that's the case, shouldn't we consider replacing the existing (xUnit.net) test-suite with just a single Expecto one?

As I see it, there's a huge benefit here: one test-suite for both native (F#) and compiled (JS headless and browser) targets.

Yes. This is the idea to write one test for both worlds. That is excatly why Zaid Ajaj made Fable.Mocha library to have the same interface as Expecto.

[moodmosaic]

The thing that does not work in Fable.Mocha is it does not support property based tests like Expecto

This shouldn't be a problem as most (if not all) of the test-suite is xUnit.net-based.

[moodmosaic]

🏓

[ThisFunctionalTom]

🏓

Sorry, is it expected from me to do something here? I am not sure how this works :-)

[moodmosaic]

It's just a 'ping' (mostly for me) to get back to this anytime soon. Sorry for any trouble caused.

[ThisFunctionalTom]

It's just a 'ping' (mostly for me) to get back to this anytime soon. Sorry for any trouble caused.

No problem. I just didn't know if I had to do something so I asked.

[moodmosaic]

Marking this pull request as draft now. The plan is to try to remove the duplication in the test code.

[ghost]

@moodmosaic I would love to get this merged. For some reason ionide gets stuck in a tight loop in the presence of double back-ticks, and makes my computer sound like it's going to achieve flight. Given that all of the xunit tests have double back-ticks everywhere this is a problem for me.

@ThisFunctionalTom are you able to rebase these changes?

[ThisFunctionalTom]

@moodmosaic I would love to get this merged. For some reason ionide gets stuck in a tight loop in the presence of double back-ticks, and makes my computer sound like it's going to achieve flight. Given that all of the xunit tests have double back-ticks everywhere this is a problem for me.

@ThisFunctionalTom are you able to rebase these changes?

Did it.

Also, we could try to upgrade fable compiler to fable 3.

[moodmosaic]

@ThisFunctionalTom, thanks for rebasing. 👍

I haven't followed Fable (3) but if that means we can run everything without having to install Node.js (and NPM) then perhaps it'll make things simpler.

@adam-becker, off the top of my head, we'd have to check the following before even consider merging:

1. The tests in this PR have all the newly added ones (for example those added in Add Tree.toString #228)
2. GitHub Actions will pick up all the tests during build(s)

[ThisFunctionalTom]

@ThisFunctionalTom, thanks for rebasing. 👍

I haven't followed Fable (3) but if that means we can run everything without having to install Node.js (and NPM) then perhaps it'll make things simpler.

I don't think we can escape Node.js 😢 because we have to run the tests with mocha test runner and it comes as npm package.

@adam-becker, off the top of my head, we'd have to check the following before even consider merging:

1. The tests in this PR have all the newly added ones (for example those added in Add Tree.toString #228)
2. GitHub Actions will pick up all the tests during build(s)

I have to check this. I will try to do it this week.

[ghost]

I don't think we can escape Node.js 😢 because we have to run the tests with mocha test runner and it comes as npm package.

@ThisFunctionalTom Isn't the point of these tests that it's flexible? According to the documentation, we should be able to run with expecto. At least in a CI setting, whatever a developer chooses to do with their local environment is up to them.

[ThisFunctionalTom]

@adam-becker The problem is that the underlying

I don't think we can escape Node.js 😢 because we have to run the tests with mocha test runner and it comes as npm package.

@ThisFunctionalTom Isn't the point of these tests that it's flexible? According to the documentation, we should be able to run with expecto. At least in a CI setting, whatever a developer chooses to do with their local environment is up to them.

Running the tests just with expecto has nothing to do with fable compiler. It is just normal F# compiler + .NET runtime.
The Fable compiler generates javascript and we should test if the generated javascript code is correct. To do it we need nodejs and mocha (unit test runner).
The new fable compiler is a .NET tool but it doesnt help us because we need to test the generated code. At least I think so.

[ghost]

Running the tests just with expecto has nothing to do with fable compiler. It is just normal F# compiler + .NET runtime.
The Fable compiler generates javascript and we should test if the generated javascript code is correct. To do it we need nodejs and mocha (unit test runner).
The new fable compiler is a .NET tool but it doesnt help us because we need to test the generated code. At least I think so.

@ThisFunctionalTom I just meant from a CI perspective, but you're right that we would still want to exercise the Fable generated code in case any bugs crop up there. As long as the project doesn't require nodejs for development I think it's still fine.

@moodmosaic I'm unsure how much you're allowed customize the build environment with github actions, but if it's anything like GitLab CI with docker then this should be trivial to support.

[ThisFunctionalTom]

Hi, sorry for the long delay.

I made following changes:

• rebased again on master
• copied changes made in Hedgehog.Tests to Hedgehog.Fable.Tests
• copied and converted new TreeTests to Hedgehog.Fable.Tests
• upgraded fable compiler to fable3 (added fable3 compiler dotnet tool)

I ran into a small issue with fable 3. Since GenTuple module is private fable 3 failed to compile 'Property.fs' file with following errors:

C:/Users/leko.tomas/private-source/fsharp-hedgehog/src/Hedgehog/Property.fs(38,8): (38,52) error FABLE: Cannot reference private members from other files: Hedgehog.GenTuple.mapSnd
C:/Users/leko.tomas/private-source/fsharp-hedgehog/src/Hedgehog/Property.fs(67,19): (67,34) error FABLE: Cannot reference private members from other files: Hedgehog.GenTuple.mapSnd
C:/Users/leko.tomas/private-source/fsharp-hedgehog/src/Hedgehog/Property.fs(79,16): (79,62) error FABLE: Cannot reference private members from other files: Hedgehog.GenTuple.mapFst
Compilation failed

I removed the private modifier from GenTuple and it works now, but we should probably discuss if this is the best solution. I am not sure why this happens. There is already a simmilar issue open in Fable repo for this.

npm is still needed for running tests in node/browser runtimes.

If you would like to merge this we should probably think/talk about exchanging Hedgehog.Tests with Hedghog.Fable.Tests because they are duplicates now. If we run Hedghog.Fable.Tests with dotnet run they are running in .NET runtime with expecto and should actually be the same tests as Hedgehog.Tests run with dotnet test (xunit runner). It would be really cool if we could do this because we would not have to maintain two separate projects with completely same tests.

To try the same tests in javascript you can:

• cd into Hedgehog.Fable.Tests directory
• npm install -> download/install javascript dependencies
• npm run build -> build all tests with fable 3
• npm run test -> run the tests in node environment
• npm run start and then open http://localhost:8085/ and wait -> run the tests in browser environment

[ghost]

Great work @ThisFunctionalTom! I'm in favor of these changes, just a few things I think we should address.

• Hedgehog.Tests should be supplanted by these changes. In other words, delete that project, and rename Hedgehog.Fable.Tests to just Hedgehog.Tests.
• Since Xunit is no longer a dependency, the functions in TestHelpers.fs should be updated to remove Xunit terminology. However, I would prefer those helpers being inlined and removed.
• The theory tests use <| to pass a lambda as the last argument, we removed usages of this operator in Remove backward pipe #276, so we should consider not reintroducing it here. I won't block on this point however.
• These tests need to run as part of CI, so changing master.yml and pull_request.yml to include these tests would be my only real requirement.

[ghost]

I was able to rebase the branch, I'll give it another look and merge.

[ghost]

@ThisFunctionalTom Thanks again! Great work.