Roadblocks on updating tests baselines

[auduchinok]

There're some things that make it more difficult to contribute to this repo than it used to be.

Surface area tests

• There're two different files need updating, the debug and release ones, despite their contents are the same
  • Let's have only one instead? It would save developers from switching the solution configuration and extra file manipulations
• The actual and expected files are in different directories
  • Let's put the actual file next to the expected one, so it's easier to compare and replace them
• The contents get reordered
  • Let's have a stable sorting, so it's easier to see the actual difference

ILVerify

• It requires non-standard shell
  • Let's make it shell-agnostic (or at least not require installing additional shells on the computer)
• It doesn't seem to work in some cases (see Cancellable: set token in more places #18283 (comment))

[T-Gro]

I think all are sensible.

[KevinRansom]

@auduchinok best not to write back to where the source code lives. It for sure impacts parallel test runs. A single file is a decent idea, the DirectoryAttribute tests have a probing scheme that allows release/debug/net472/netcore to produce varying output we can certainly do something like that for surface area.

I believe FSharp.Core has different debug out, at least it used to. So multiple possibilities are probably still useful.

[KevinRansom]

The contents get reordered

• Let's have a stable sorting, so it's easier to see the actual difference

This is a bit wierd since we do sort before producing the actual file:

fsharp/tests/FSharp.Test.Utilities/SurfaceArea.fs

Line 56 in 5d0812f

let actual =

So this needs looking at, but yes sorting should be done and why it fails to be stable understood.

[KevinRansom]

There're some things that make it more difficult to contribute to this repo than it used to be.

Surface area tests

• There're two different files need updating, the debug and release ones, despite their contents are the same

  • Let's have only one instead? It would save developers from switching the solution configuration and extra file manipulations

• The actual and expected files are in different directories

  • Let's put the actual file next to the expected one, so it's easier to compare and replace them

• The contents get reordered

  • Let's have a stable sorting, so it's easier to see the actual difference

ILVerify

• It requires non-standard shell

  • Let's make it shell-agnostic (or at least not require installing additional shells on the computer)

• It doesn't seem to work in some cases (see Cancellable: set token in more places #18283 (comment))

Yes, this is annoying.

Now that IL verify has been added as part of the test framework we should consider making these verifications standard testcases rather than an explicit CI path.

We should also try to eliminate all of the validation errors that exist in the ilverify baselines, either by ensuring ilverify doesn't produce the errors if they are truly "not an issue" or fixing the compiler codegen if they are valid issues.

[auduchinok]

A single file is a decent idea, the DirectoryAttribute tests have a probing scheme that allows release/debug/net472/netcore to produce varying output we can certainly do something like that for surface area.

I believe FSharp.Core has different debug out, at least it used to.

Let's have a single file at least for FCS then, as it's updated quite frequently and doesn't need the second file for now. If it ever changes we'll reconsider it for sure

[auduchinok]

I'm trying to update the baselines in #18311.

I've opened up a CMD instance (it seems to be the shell in the tests) and executed

set TEST_UPDATE_BSL=1

as suggested in the dev guide.

I've then run the tests using this command:

build.cmd -compressallmetadata -buildnorealsig -testDesktop -configuration Release

The two tests have indeed failed, but the baselines have not been updated. Is there something I'm missing?

What is the easiest way to run these tests isolated?

[majocha]

dotnet test FSharp.sln -c Release -f net9.0 --filter "SurfaceArea" should work.
with TEST_UPDATE_BSL Surface area tests will just overwrite the baselines, which is nice, because it will show up in git changes immediately.

EDIT: I see you probably mean typecheck error baselines. I don't think those update with TEST_UPDATE_BSL.