Uh oh!
There was an error while loading. Please reload this page.
Split RunFileTests into multiple files to match main branch structure - #54037
Conversation
Splits the monolithic RunFileTests.cs into: - RunFileTestBase.cs: shared base class, fields, helpers - RunFileTests_General.cs: general file path, stdin, project tests - RunFileTests_BuildOptions.cs: build options, binary logs, verbosity tests - RunFileTests_BuildCommands.cs: restore, build, publish, pack, clean tests - RunFileTests_Directives.cs: directive tests (define, package, sdk, ref, include) - RunFileTests_CscOnlyAndApi.cs: csc-only, up-to-date, API tests This matches the split structure on main to minimize merge conflicts when flowing changes between branches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds the fixture class that: - Copies NuGet.config to the runfile base directory - Ensures a simple app runs fully with MSBuild before other tests so packages like ILLink.Tasks are restored and csc-only optimization works Adapted for xUnit v2 (Task instead of ValueTask). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port fix from main branch - the tool exec commands in Pack and Pack_CustomPath tests need an isolated NUGET_PACKAGES directory to avoid picking up cached packages from other test runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR splits the previously monolithic RunFileTests test coverage into multiple focused test files, aligning the structure with main branch to reduce merge conflicts when flowing changes between branches.
Changes:
- Introduces
RunFileTestBase(plus fixture) to centralize shared test helpers/data. - Splits run-file test coverage into separate files for general behavior, directives, build options/commands, and CSC-only/API scenarios.
- Updates
DotnetProjectConvertTeststo reference the new shared helpers onRunFileTestBase.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/dotnet.Tests/CommandTests/Run/RunFileTestBase.cs | Adds shared base class + fixture/helpers used by the split test classes. |
| test/dotnet.Tests/CommandTests/Run/RunFileTests_General.cs | General file-path/stdin/project-ambiguity test coverage moved here. |
| test/dotnet.Tests/CommandTests/Run/RunFileTests_Directives.cs | Directive-focused test coverage (package/sdk/project/ref/include/exclude/etc.). |
| test/dotnet.Tests/CommandTests/Run/RunFileTests_CscOnlyAndApi.cs | CSC-only optimization and run-api coverage moved here. |
| test/dotnet.Tests/CommandTests/Run/RunFileTests_BuildOptions.cs | Build-option behavior (binlog/verbosity/props/etc.) moved here. |
| test/dotnet.Tests/CommandTests/Run/RunFileTests_BuildCommands.cs | Build-command behavior (restore/build/publish/pack/clean/etc.) moved here. |
| test/dotnet.Tests/CommandTests/Project/Convert/DotnetProjectConvertTests.cs | Updates helper references from the old monolithic type to RunFileTestBase. |
| new DotnetCommand(Log, "Program.cs", "arg1", "arg2") | ||
| .WithWorkingDirectory(testInstance.Path) | ||
| .Execute() | ||
| .Should().Pass() | ||
| .And.HaveStdOut(""" | ||
| echo args:arg1;arg2 | ||
| Hello from Program | ||
| Release config | ||
| """); | ||
There was a problem hiding this comment.
This DotnetCommand(Log, "Program.cs", "arg1", "arg2") assertion block is duplicated verbatim earlier in the same test method, adding extra execution time without covering new behavior. Remove the duplicate invocation or vary it to validate a distinct scenario (e.g., caching/up-to-date behavior) with an explanatory comment.
| newDotnetCommand(Log,"Program.cs","arg1","arg2") | |
| .WithWorkingDirectory(testInstance.Path) | |
| .Execute() | |
| .Should().Pass() | |
| .And.HaveStdOut(""" | |
| echoargs:arg1;arg2 | |
| Hellofrom Program | |
| Releaseconfig | |
| """); |
| /// <summary> | ||
| /// <c>dotnet run -- -</c> should NOT read the C# file from stdin, | ||
| /// the hyphen should be considred an app argument instead since it's after <c>--</c>. |
There was a problem hiding this comment.
Typo in XML doc comment: "considred" should be "considered".
| /// the hyphen should be considred an app argument instead since it's after <c>--</c>. | |
| /// the hyphen should be considered an app argument instead since it's after <c>--</c>. |
| } | ||
| /// <summary> | ||
| /// <c>Directory.Build.props</c> doesn't have any effect on <c>dotnet run -</c>. |
There was a problem hiding this comment.
The XML doc comment here mentions Directory.Build.props having no effect on dotnet run -, but the test body is about #:project handling from stdin (project reference resolution). Update the summary to describe what this test actually verifies so future readers can find the right coverage.
| /// <c>Directory.Build.props</c> doesn't have any effect on <c>dotnet run -</c>. | |
| /// <c>dotnet run -</c> resolves <c>#:project</c> references from stdin using MSBuildStartupDirectory-based paths, but not relative paths. |
| public abstract class RunFileTestBase(ITestOutputHelper log) : SdkTest(log), IClassFixture<RunFileTestFixture> | ||
| { |
There was a problem hiding this comment.
RunFileTestBase implements IClassFixture<RunFileTestFixture>, which means RunFileTestFixture.InitializeAsync() runs once per derived test class (now several files), repeating a relatively expensive dotnet run - restore/warmup multiple times. Consider switching this to a shared collection fixture (ICollectionFixture + [Collection]) or adding a static one-time guard inside the fixture to ensure the warmup runs only once per test assembly run.
Uh oh!
There was an error while loading. Please reload this page.
Splits the monolithic RunFileTests.cs into:
This matches the split structure on main to minimize merge conflicts when flowing changes between branches.