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.