dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.91k stars 785 forks source link

Tests are broken in main in FSharp.Compiler.Service solution #17656

Open auduchinok opened 2 months ago

auduchinok commented 2 months ago

There're broken tests in FSharp.Compiler.Service solution, seemingly due to the nullability changes in the compiler and the corresponding changes in FSharp.Core.

The tests work in FSharp.sln. I assume the only difference is the FSharp.Core reference? There shouldn't be need to use the same reference for building/running FCS and for analysis in tests. The tests should be able to use any referenced FSharp.Core, independently of the one used by FCS itself in runtime.

vzarytovskii commented 2 months ago

I assume the only difference is the FSharp.Core reference?

Yes, this is the reason for it.

The tests should be able to use any referenced FSharp.Core, independently of the one used by FCS itself in runtime.

I am not yet sure how it's going to work, since it will cause dependency conflict, since when building test project, it will cause the build of FCS, which by default uses FSharp.Core package, but if we force test project to use FSharp.Core, the version will differ.

I will need to think more about it/

auduchinok commented 2 months ago

I am not yet sure how it's going to work, since it will cause dependency conflict, since when building test project, it will cause the build of FCS, which by default uses FSharp.Core package

The FSharp.Core assembly passed as a reference to FCS during test data analysis should not in any way depend on what FSharp.Core was used to build the test project itself.

edgarfgp commented 2 months ago

I imagine you already know this . But <BUILDING_USING_DOTNET>false</BUILDING_USING_DOTNET> in Directory.Build.props. will allow to run tests as before.

Screenshot 2024-09-04 at 15 40 26
vzarytovskii commented 1 month ago

I imagine you already know this . But <BUILDING_USING_DOTNET>false</BUILDING_USING_DOTNET> in Directory.Build.props. will allow to run tests as before.

Screenshot 2024-09-04 at 15 40 26

This shouldn't probably be used, it can cause a bunch of issues when mixing arcade and non-arcade builds, FSHARPCORE_USE_PACKAGE should be used instead.

@auduchinok I don't think we can provide an arbitrary DLL (from project! to the unit tests. Only one which they were built with, without modifying build process too much.

Will the property workaround work meanwhile?

auduchinok commented 1 month ago

@auduchinok I don't think we can provide an arbitrary DLL (from project! to the unit tests. Only one which they were built with, without modifying build process too much. Will the property workaround work meanwhile?

I'll try it when back to working with the compiler. Hopefully, soon 🙂