dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.04k stars 4.03k forks source link

Microsoft.CodeAnalysis.CSharp.CommandLine.UnitTests.CommandLineTests.ArgumentParsing is flaky #58077

Open runfoapp[bot] opened 2 years ago

runfoapp[bot] commented 2 years ago

Note: the test is skipped in some legs, so the runinfo count below isn't representative of whether it was fixed.

Runfo Tracking Issue: Microsoft.CodeAnalysis.CSharp.CommandLine.UnitTests.CommandLineTests.ArgumentParsing is flaky Build Definition Kind Run Name
Build Result Summary Day Hit Count Week Hit Count Month Hit Count
0 0 0
Youssef1313 commented 2 years ago

I took a quick look at the code. The try/finally block looks suspicious to me. An exception could have been thrown causing the flow to silently go into the finally block. My guess is that an exception was thrown before the part of the code that reports the expected diagnostics.

https://github.com/dotnet/roslyn/blob/649315781b4e4f08f35e92c4b98a32295d39df79/src/Compilers/Core/Portable/CommandLine/CommandLineParser.cs#L1239-L1311

Specifically, the FileUtilities.ResolveRelativePath(resolvedPath, baseDirectory); call.

Note that this is almost only guesses. May or may not be correct.

Youssef1313 commented 2 years ago

It seems like my guess isn't correct. I removed the try/finally and the test still fails with the same failure (not crashing with an exception as I was expecting)

Youssef1313 commented 2 years ago

My investigations:

https://github.com/dotnet/roslyn/blob/649315781b4e4f08f35e92c4b98a32295d39df79/src/Compilers/Core/Portable/CommandLine/CommandLineParser.cs#L1231-L1236

there are the following variables and their values:

Variable Value
directory "/"
baseDirectory "/tmp/RoslynTests"
resolvedDirectoryPath "/"

I hope this provides some entry point for more investigation.

jcouv commented 2 years ago

@Youssef1313 Have you been able to repro this by any chance? What does the failure look like? OP doesn't provide much info especially since the CI legs that failed are now out of range of the tracking tool...

Youssef1313 commented 2 years ago

@jcouv I couldn't reproduce locally, but I'd expect a repro to be like:

The questions are:

Youssef1313 commented 2 years ago

@jcouv The test no longer fails on CI. Whatever thing was created the /1 file has gone away. But it would be good to try to reproduce on a Linux machine locally and see if it should have failed with the /1 file in the first place.

jcouv commented 2 years ago

@333fred Are you still set up on linux? Could you try the repro provided above to see if we still have a bug?

jcouv commented 2 years ago

@333fred Are you still set up on linux? Could you try the https://github.com/dotnet/roslyn/issues/58077#issuecomment-1070349349 provided above to see if we still have a bug?

333fred commented 2 years ago

That does indeed reproduce the bug. Steps:

  1. Change ArgumentParsing to Fact
  2. sudo touch /1 && sudo chown <your_username>:<your_username> /1
  3. dotnet test src/Compilers/CSharp/Test/CommandLine --filter "DisplayName~ArgumentParsing"
jcouv commented 2 years ago

Thanks @333fred for the additional linux repro details. I was able to repro and have a fix pending.