dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.91k stars 4.64k forks source link

Can we avoid resetting all env vars for some remotely executed tests? #100505

Open AndyAyersMS opened 5 months ago

AndyAyersMS commented 5 months ago

The environment clearing done here:

https://github.com/dotnet/runtime/blob/4039b4561595166f75577080ee784bc7f53e10a4/src/libraries/System.Memory/tests/Span/StringSearchValues.cs#L554-L563

(and likely elsewhere) makes impossible to tunnel DOTNET options through the test layers to enable various JIT diagnostics and debugging techniques.

cc @MihaZupan @dotnet/jit-contrib

AndyAyersMS commented 5 months ago

Also blocks various testing modes: gcstress, jitstress, SPMI collection, etc.

MihaZupan commented 5 months ago

For the SearchValues tests I picked this up from CultureInfo tests. The clearing likely isn't needed here

AndyAyersMS commented 5 months ago

@BruceForstall pointed me at this, which seems like a good pattern to emulate:

https://github.com/dotnet/runtime/blob/4039b4561595166f75577080ee784bc7f53e10a4/src/libraries/System.Runtime/tests/System.Globalization.Tests/CultureInfo/CultureInfoCurrentCulture.cs#L182-L195

There are only 6 hits for Environment.Clear(); so perhaps I can go audit them all and change them over to this pattern.

jkotas commented 5 months ago

I think both these patterns are questionable.

In both of these cases, the code wants to clear or preserve environment variables that affect process globalization settings. The code should clear the specific environment variables - it is a small finite number - and leave everything else intact.

It would be nice to factor this out into a helpers under src\libraries\Common so that we have just one place to update if we discover new environment variables that affect globalization.

AndyAyersMS commented 4 months ago

The code should clear the specific environment variables - it is a small finite number - and leave everything else intact.

Can somebody help me figure out what needs to be cleared?

jkotas commented 4 months ago

I think clearing LANG and all env variables that start with DOTNET_SYSTEM_GLOBALIZATION prefix should do it.

AndyAyersMS commented 1 month ago

Still want to do this, but not a must-have for 9.0