Fix launch settings stderr test assertions and container digest validation#53906
Fix launch settings stderr test assertions and container digest validation#53906marcpopMSFT wants to merge 5 commits intomainfrom
Conversation
…ation - Update test assertions in GivenDotnetRunBuildsCsProj.cs, GivenDotnetRunBuildsVbProj.cs, and RunCommandTests.cs to expect the 'Using launch settings from' message on stderr instead of stdout, matching the production change in commit 1ebcc5e. - Fix VB test to use 'My Project' path instead of 'Properties' for launchSettings.json, matching VB project conventions. - Add strict sha256 digest validation in ContentStore.PathForDescriptor using an anchored regex requiring exactly 64 hex characters, replacing the permissive DigestRegexp that accepted 32+ characters unanchored. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates tests and container digest validation to match recent production behavior changes, unblocking CI by aligning assertions/validation with current dotnet run output streams and stricter digest formatting.
Changes:
- Update
dotnet runtests to assert the “Using launch settings from …” message on stderr (and fix VB launchSettings path). - Tighten container digest validation to require
sha256:<64 hex>format.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/dotnet.Tests/CommandTests/Run/RunCommandTests.cs | Adjusts launch settings assertion to expect output on stderr. |
| test/dotnet.Tests/CommandTests/Run/GivenDotnetRunBuildsVbProj.cs | Moves launch settings assertions to stderr and fixes VB launchSettings path. |
| test/dotnet.Tests/CommandTests/Run/GivenDotnetRunBuildsCsProj.cs | Moves multiple launch settings assertions to stderr. |
| src/Containers/Microsoft.NET.Build.Containers/ContentStore.cs | Adds strict sha256 digest regex validation for descriptor paths. |
Comments suppressed due to low confidence (1)
src/Containers/Microsoft.NET.Build.Containers/ContentStore.cs:45
- The new digest validation regex is case-sensitive for the "sha256:" prefix, but other helpers in this assembly (e.g., DigestUtils.GetShaFromDigest) accept the algorithm prefix case-insensitively. Consider making the check case-insensitive (or normalizing the prefix) so valid digests like "SHA256:<64 hex>" don’t start failing unexpectedly.
private static readonly Regex s_sha256DigestRegex = new(@"^sha256:[0-9A-Fa-f]{64}$", RegexOptions.Compiled);
public static string PathForDescriptor(Descriptor descriptor)
{
string digestString = descriptor.Digest;
if (!s_sha256DigestRegex.IsMatch(digestString))
{
throw new ArgumentException($"Invalid digest: {digestString}", nameof(descriptor.Digest));
}
string digestValue = digestString.Substring("sha256:".Length);
| .And.HaveStdOutContaining("ARGS=arg1,arg2,arg3"); | ||
|
|
||
| cmd.StdErr.Should().BeEmpty(); | ||
| cmd.StdErr.Should().Contain("Using launch settings from"); |
There was a problem hiding this comment.
The test now only checks that stderr contains the substring "Using launch settings from". To make the assertion resilient and ensure the message moved streams (and isn’t duplicated), assert against the full formatted message using the known launchSettingsPath, and also assert it is not present on stdout.
| cmd.Should().Pass() | ||
| .And.NotHaveStdOutContaining(string.Format(CliCommandStrings.UsingLaunchSettingsFromMessage, launchSettingsPath)) | ||
| .And.HaveStdOutContaining("First"); | ||
|
|
||
| cmd.StdErr.Should().BeEmpty(); | ||
| cmd.StdErr.Should().Contain( | ||
| string.Format(CliCommandStrings.UsingLaunchSettingsFromMessage, launchSettingsPath)); | ||
| } |
There was a problem hiding this comment.
In this test the previous assertion that the "Using launch settings from..." message is not written to stdout was removed. Since the intent is specifically to validate the stream change, keep an explicit stdout-negative assertion to catch accidental duplication (stdout + stderr).
|
FYI #53905 supersedes the stderr fix. |
- Fix DotnetAddPostActionProcessor.ProcessInternal: when targetFiles is specified and GetConfiguredFiles returns empty, try FindExistingTargetFiles before TryFindProjects to avoid resolving stale parent .csproj files - Remove -bl flag from CscOnly_CompilationDiagnostics test that caused false failures on Linux CI when NuGet cache files are missing - Add regression test for PostAction fix with conflicting parent project Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… from #53933 and additional test fixes # Conflicts: # src/Containers/Microsoft.NET.Build.Containers/ContentStore.cs # src/Containers/Microsoft.NET.Build.Containers/DigestUtils.cs # src/Containers/Microsoft.NET.Build.Containers/LocalDaemons/DockerCli.cs # src/Containers/Microsoft.NET.Build.Containers/ManifestV2.cs # src/Containers/Microsoft.NET.Build.Containers/Registry/Registry.cs # test/dotnet.Tests/CommandTests/Run/GivenDotnetRunBuildsCsProj.cs # test/dotnet.Tests/CommandTests/Run/GivenDotnetRunBuildsVbProj.cs # test/dotnet.Tests/CommandTests/Run/RunCommandTests.cs
3d3cf9b to
1527e3b
Compare
Summary
Fixes CI test failures on main caused by mismatched test assertions after recent production changes.
Launch Settings stderr (18+ tests affected)
Commit 1ebcc5e changed the 'Using launch settings from' message to write to stderr instead of stdout, but only updated tests in one file. This PR updates the remaining three test files:
Container Digest Validation (6 tests affected)
\ContentStore.PathForDescriptor\ used the unanchored \DigestRegexp\ ({32,}\ hex chars) which accepted invalid digests like 63-char hex strings. Replaced with a strict anchored regex requiring exactly \sha256:<64 hex chars>.
Testing
Build verified locally (
et472\ and
et11.0\ targets). These fixes address failures observed in Build 1374751.