Clean up test discovery #76663
Clean up test discovery #76663
Conversation
Make a few small changes here: 1. Added a README.md to explain the purpose of `TestDiscoveryWorker` 2. Moved to using standard command line arguments so the tool can be run locally more easily
dotnet-issue-labeler
bot
added
Area-Infrastructure
untriaged
|
@dotnet/roslyn-infrastructure PTAL |
| .. Directory.EnumerateDirectories(artifactsDir, "*.UnitTests"), | ||
| .. Directory.EnumerateDirectories(artifactsDir, "*.IntegrationTests"), | ||
| .. Directory.EnumerateDirectories(artifactsDir, "RunTests") | ||
| ]; |
There was a problem hiding this comment.
what black magic is this...
There was a problem hiding this comment.
Just trying to creat a List<string> vs a lazy evaluated IEnumerable<string>. No functional changes here.
| List<string> directories = | ||
| [ | ||
| Path.Combine(sourceDirectory, "eng"), | ||
| Path.Combine(sourceDirectory, "artifacts", "VSSetup"), |
There was a problem hiding this comment.
I guess the previous !isUnix check is not needed here?
There was a problem hiding this comment.
No. That check was basically replaced with the Directory.Exists check. I like the latter better cause I can run the tools locally without having a 100% full roslyn build.
| } | ||
|
|
||
| Console.WriteLine($"{testsToWrite.Count} found"); | ||
|
|
||
| using var fileStream = new FileStream(outputFilePath, FileMode.OpenOrCreate, FileAccess.Write, FileShare.None); |
There was a problem hiding this comment.
If the file already exists, will this code do the right thing? I would expect existing file contents to get replaced with the new contents?
| catch (OptionException e) | ||
| { | ||
| Console.WriteLine(e.Message); | ||
| options.WriteOptionDescriptions(Console.Out); | ||
| return ExitFailure; | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| // Write the exception details to stderr so the host process can pick it up. |
There was a problem hiding this comment.
It doesn't look like we write to stderr anymore, i.e., the comment is incorrect.
There was a problem hiding this comment.
Good catch. Will fix this on the next change.
| @@ -4,12 +4,14 @@ | |||
| <OutputType>Exe</OutputType> | |||
| <TargetFrameworks>$(NetRoslyn);net472</TargetFrameworks> | |||
| <Nullable>enable</Nullable> | |||
| <SignAssembly>false</SignAssembly> | |||
There was a problem hiding this comment.
The Mono.Options reference isn't signed hence this can't be either since it uses it.
Make a few small changes here:
TestDiscoveryWorker