Moving Libraries Control tests to NUnit

[thinkbeforecoding]

No description provided.

[dnfclas]

All CLA requirements met.

[forki]

Awesome

[thinkbeforecoding]

@forki if we all migrate some of those tests, it could quick 😄

[thinkbeforecoding]

I started with a small one, to check I got it correctly

[forki]

if we all migrate some of those tests, it could quick

Yes that's the idea. Crowdsourcing ftw.

Also it's always nice to have new contributors

[thinkbeforecoding]

@forki If moving more tests, is it better to add them to this PR or to create a new PR for each ?

[sergey-tihon]

@thinkbeforecoding I vote for new PRs for each.
Smaller PRs -> fasters merge -> master in better shape -> easier to track what is done and what is missing.

[sergey-tihon]

Looks like the similar error as here #7210 (comment)

[cartermp]

Looks good overall, some minor changes for comments (in case we have to go in later and determine what's going on), and some questions for @dsyme

[forki]

It's definitely better to have multiple small PRs with up to ~10 tests each (or fewer). Bigger PRs are harder to review. We already saw that some became so big that they never went in. Sergey Tihon <notifications@github.com> schrieb am So., 14. Juli 2019, 17:26:

…

@thinkbeforecoding <https://github.com/thinkbeforecoding> I vote for new PRs for each. Smaller PRs -> fasters merge -> master in better shape -> easier to track what is done and what is missing. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7234?email_source=notifications&email_token=AAAOANHSKHGKPFOTYUI37O3P7NATVA5CNFSM4IDQMQXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ4HS4Q#issuecomment-511211890>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAOANAIPZSW6GK2KUJHTFDP7NATVANCNFSM4IDQMQXA> .

[cartermp]

Additionally, it might be helpful to rename the folder to Async and the main file to AsyncTests, since the "control" name has since been deprecated and each piece of it was broken into a separate file in FSharp.Core.

[thinkbeforecoding]

On netcore, tests are failing, with a

Method not found: 'Microsoft.FSharp.Control.FSharpAsync`1<...>'

It seems it's because CompilerAssert.CompileExeAndRun isn't prepared to run with build FSharp.Core.dll

[thinkbeforecoding]

For folder, something like:

Libraries/
    + Async/
          + AsyncTests.fs

?

[cartermp]

@thinkbeforecoding Yep!

[cartermp]

Also @KevinRansom the failure is interesting, the project file looks fine to me though: https://github.com/dotnet/fsharp/blob/master/tests/fsharp/FSharpSuite.Tests.fsproj

[thinkbeforecoding]

Looks like the similar error as here #7210 (comment)

Yes looks like the same issue

[thinkbeforecoding]

@cartermp It seems the error occurs when running the exe built by the test when it uses FSharp.Core.

[thinkbeforecoding]

I managed to reproduce the error locally.. investigation under way

[thinkbeforecoding]

@cartermp could it be due to the fact that there is no .deps.json file created along the generated exe ?

[thinkbeforecoding]

adding a .deps.json doesn't seem to solve the issue 🤨

[forki]

I assume we have to copy the self-built fsharp.core over into execution folder. @TIHan any chance you can take a look and help to unblock so that we don't lose momentum? Thanks Jérémie Chassaing <notifications@github.com> schrieb am So., 14. Juli 2019, 19:22:

…

adding a .deps.json doesn't seem to solve the issue 🤨 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7234?email_source=notifications&email_token=AAAOANE7SDUN7FPKJF7INHTP7NOGDA5CNFSM4IDQMQXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ4JVXI#issuecomment-511220445>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAOANHW5YX72BA5XLMVHKDP7NOGDANCNFSM4IDQMQXA> .

[thinkbeforecoding]

I tried copying the netsandard1.6 version of Fsharp.Core along the exe.. But getting the same error.

And actually the the fsharp.core is already present during the test. as well as a .runtimeconfig.json that seem valid.

When executing from the cmd line dotnet C:\Users\x\AppData\Local\Temp\tmpECF1.exe manually I get the same error, so the problem is not in the exe invocation code.

[thinkbeforecoding]

I've also inspected the .exe and FSharp.Core.dll, and they seem ok and the method exists in FSharp.Core.dll ...

[forki]

Maybe it's binding itself against one in gac? I mean everything works locally Jérémie Chassaing <notifications@github.com> schrieb am Mo., 15. Juli 2019, 10:40:

…

I've also inspected the .exe and FSharp.Core.dll, and they seem ok and the method exists in FSharp.Core.dll ... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7234?email_source=notifications&email_token=AAAOANCP4VORFLXDNV5NGMDP7QZWBA5CNFSM4IDQMQXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ5BRLI#issuecomment-511318189>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAOANCBG5U27JRZVNJULDTP7QZWBANCNFSM4IDQMQXA> .

[thinkbeforecoding]

It's working with net framework but not with dotnet core on windows...
To test, is swapped the fx in from test project to put netcorapp2.1 first. This way tests are run in netcoreapp2.1 and failing

[thinkbeforecoding]

@cartermp @TIHan ? Any idea 😶 ?

[thinkbeforecoding]

Still trying to understand...
In the test compiled code, it fails on Async.RunSynchronously, but if I replace it with Async.Start, it doesn't cause the method not found exception...

[thinkbeforecoding]

I could attach a debugger inside the generated exe, and the attached FSharp.Core is the correct on, and has a method:

T RunSynchronously[T](Microsoft.FSharp.Control.FSharpAsync`1[T], Microsoft.FSharp.Core.FSharpOption`1[System.Int32], Microsoft.FSharp.Core.FSharpOption`1[System.Threading.CancellationToken])

and the error says it tries to find:

!!0 Microsoft.FSharp.Control.FSharpAsync.RunSynchronously(Microsoft.FSharp.Control.FSharpAsync`1<!!0>, Microsoft.FSharp.Core.FSharpOption`1<System.Int32>, Microsoft.FSharp.Core.FSharpOption`1<System.Threading.CancellationToken>)

where !!0 seems to be the generic type. The only difference I see is that the second one does not show that the method is generic...

[thinkbeforecoding]

The problem definitely occurs only with Generic methods !

[thinkbeforecoding]

this is the ILSpy of the line that cause the exception:

	IL_0015: call !!0 [FSharp.Core]Microsoft.FSharp.Control.FSharpAsync::RunSynchronously<valuetype [System.Private.CoreLib]System.Int32[]>(class [FSharp.Core]Microsoft.FSharp.Control.FSharpAsync`1<!!0>, class [FSharp.Core]Microsoft.FSharp.Core.FSharpOption`1<valuetype [System.Private.CoreLib]System.Int32>, class [FSharp.Core]Microsoft.FSharp.Core.FSharpOption`1<valuetype [System.Private.CoreLib]System.Threading.CancellationToken>)
	IL_001a: ret

[thinkbeforecoding]

This is the module information for the loaded FSharp.Core while running:

FSharp.Core.dll
FSharp.Core.dll
C:\Users\x\AppData\Local\Temp\FSharp.Core.dll
No
N/A
Symbols loaded. C:\Development\fsharp\artifacts\obj\FSharp.Core\Debug\netstandard1.6\FSharp.Core.pdb
4
42.42.42.42424
17/09/2105 11:10
00000217689E0000-0000021768D10000
[24384] dotnet.exe
[1] clrhost

The module timestamp is really strange.

[cartermp]

@KevinRansom any ideas?

[thinkbeforecoding]

We managed with @Tarmil to make a minimal repro 👍

This is passing:

        CompilerAssert.CompileExeAndRun
            """
module M
let v = List.head [1;2]
()
            """

and this is not passing:

    CompilerAssert.CompileExeAndRun
          """
module M
let v = List.item 1 [1;2]
()
          """

Any call to an FSharp.Core method that takes an Int32 raise the 'MissingMethodException' error.
It's been tested with exit RunSynchronously (takes an optional int timeout), and List.item (takes an int).
Other tested methods that dont take an int (Async.Ignore, Async.Start, List.head) work correctly.

Is there a mismatch in the corelib references ?

[KevinRansom]

@cartermp do the changes meet your expectations?