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

[runfoapp]

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]

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.

roslyn/src/Compilers/Core/Portable/CommandLine/CommandLineParser.cs

Lines 1239 to 1311 in 6493157

	try
	{
	bool yielded = false;
	// NOTE: Directory.EnumerateFiles(...) surprisingly treats pattern "." the
	// same way as "*"; as we don't expect anything to be found by this
	// pattern, let's just not search in this case
	pattern = pattern.Trim(s_searchPatternTrimChars);
	bool singleDotPattern = string.Equals(pattern, ".", StringComparison.Ordinal);
	if (!singleDotPattern)
	{
	while (true)
	{
	string? resolvedPath = null;
	try
	{
	if (enumerator == null)
	{
	enumerator = EnumerateFiles(resolvedDirectoryPath, pattern, searchOption).GetEnumerator();
	}
	if (!enumerator.MoveNext())
	{
	break;
	}
	resolvedPath = enumerator.Current;
	}
	catch
	{
	resolvedPath = null;
	}
	if (resolvedPath != null)
	{
	// just in case EnumerateFiles returned a relative path
	resolvedPath = FileUtilities.ResolveRelativePath(resolvedPath, baseDirectory);
	}
	if (resolvedPath == null)
	{
	errors.Add(Diagnostic.Create(MessageProvider, (int)MessageProvider.FTL_InvalidInputFileName, path));
	break;
	}
	yielded = true;
	yield return resolvedPath;
	}
	}
	// the pattern didn't match any files:
	if (!yielded)
	{
	if (searchOption == SearchOption.AllDirectories)
	{
	// handling /recurse
	GenerateErrorForNoFilesFoundInRecurse(path, errors);
	}
	else
	{
	// handling wildcard in file spec
	errors.Add(Diagnostic.Create(MessageProvider, (int)MessageProvider.ERR_FileNotFound, path));
	}
	}
	}
	finally
	{
	if (enumerator != null)
	{
	enumerator.Dispose();
	}
	}

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

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

[Youssef1313]

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]

My investigations:

roslyn/src/Compilers/Core/Portable/CommandLine/CommandLineParser.cs

Lines 1231 to 1236 in 6493157

	string? directory = PathUtilities.GetDirectoryName(path);
	string pattern = PathUtilities.GetFileName(path);
	var resolvedDirectoryPath = string.IsNullOrEmpty(directory) ?
	baseDirectory :
	FileUtilities.ResolveRelativePath(directory, baseDirectory);

there are the following variables and their values:

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

• I'm not sure if resolvedDirectoryPath is supposed to be "/" or not. This comes from FileUtilities.ResolveRelativePath(directory, baseDirectory).
• There is an empty file named "1" in "/" directory. I don't think this is something expected (but I don't know for sure). I don't know what's writing this file. (If this file didn't exist, then the expected ERR_FileNotFound diagnostic is supposed to be reported)

I hope this provides some entry point for more investigation.

[jcouv]

@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]

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

• Create an empty file named 1 in / (Linux root directory)
• Run the test. It should fail.

The questions are:

• Does the existence of such a file expected to affect the test?
• Why it existed on CI in the first place?

[Youssef1313]

@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]

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

[jcouv]

@333fred Are you still set up on linux? Could you try the #58077 (comment) provided above to see if we still have a bug?

[333fred]

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]

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