Fake Context seems to fail in the test-suite

[matthid]

Description

See https://github.com/fsharp/FAKE/pull/2131/files#r223829937

Repro steps

1. Uncomment the commented line
2. Run the tests

Expected behavior

All tests (or at least the ones in the process module) should start to trace the executed processes.

Actual behavior

Half of the tests start to fail.

Known workarounds

None

[Viridovics]

Hello, @matthid !

I guess that's what happened.

We have

• let private fake_data = new AsyncLocal<System.Collections.Concurrent.ConcurrentDictionary<string, obj>>(). fake_data is lazy initialized with function getDataDict;
• Process.setEnableProcessTracing implicitly uses getDataDict;
• All tests are started as Async;
• Function let withFakeContext name f contains potential race condition (withFakeContext work implicitly with global variable fake_data: setExecutionContext and removeExecutionContext). Some test may potential remove execution context another test.

The common state of tests execution can be represented:
Control flow without Process.setEnableProcessTracing

But if we add Process.setEnableProcessTracing true before yield then getDataDict is executed before many async tests because Process.setEnableProcessTracing true is unit in generated sequence.
And now we have situation:
Control flow with Process.setEnableProcessTracing

Therefore part of tests start working with common initialized AsyncLocal fake_data. And consequently race condition of withFakeContext will be appeared.

It's easy to check: modify function getDataDict and start tests with/without Process.setEnableProcessTracing true.

let private getDataDict() =
  let value = fake_data.Value
  if isNull value then
    let l = new System.Collections.Concurrent.ConcurrentDictionary<string, obj>()
    fake_data.Value <- l
    printfn "Create new context dictionary"
    l
  else
    printfn "Use old context dictionary"
    value

I suggest solutions:

• add synchronization root for function withFakeContext (withFakeContext tests will stop being parallel) sample;
• add setup function for "Fake.Core.Process.Tests" (example);
• add analogue as withFakeContext - withEnabledProcessTracing.

Or any of your decision =)

[matthid]

This sounds very reasonable, thanks a lot for the analysis. Will take me a couple of days to process, but it is very much appreciated. I guess I'd like to have the most robust solution for people writing tests, ideally without hurting parallelism... So the last two options sound good.

One other reason I tracked this was that I was unsure if this might have any impact in fake script code, but from your analysis I'd assume it doesn't

Thanks