Repository navigation
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c8b91f0-8d92-4828-a52c-1fc9e63fe773
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c8b91f0-8d92-4828-a52c-1fc9e63fe773
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c8b91f0-8d92-4828-a52c-1fc9e63fe773
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c8b91f0-8d92-4828-a52c-1fc9e63fe773
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c8b91f0-8d92-4828-a52c-1fc9e63fe773
|
Azure Pipelines: Successfully started running 2 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Windows path handling can prevent builds or miss logs, Helix retention is bypassed, and scenario-level regression coverage is missing.
4 open issues
What changed in this PR
Adds opt-in scenario-build binlog capture to the SDK test harness, separating logs produced by tests from the runner’s build log.
Changes:
- Adds
--test-binlogs, per-run retention, and artifact counts. - Instruments supported build wrappers with command-level opt-out.
- Updates runner tests and contributor guidance.
| File | Description |
|---|---|
| test/Microsoft.NET.TestFramework/Utilities/TestPathUtility.cs | Adds cross-platform path helpers. |
| test/Microsoft.NET.TestFramework/Utilities/FileUtility.cs | Adds file hashing for change detection. |
| test/Microsoft.NET.TestFramework/Commands/TestCommand.cs | Integrates capture into command execution. |
| test/Microsoft.NET.TestFramework/Commands/TestBinlogCapture.cs | Implements logger discovery and artifact retention. |
| test/Microsoft.NET.TestFramework/Commands/MSBuildCommand.cs | Enables capture support. |
| test/Microsoft.NET.TestFramework/Commands/DotnetBuildCommand.cs | Enables capture support. |
| test/Microsoft.NET.Infrastructure.Tests/RunTestsScriptTests.cs | Tests runner options and reporting. |
| test/AGENTS.md | Documents capture configuration and opt-out. |
| scripts/RunTests.cs | Adds capture option and artifact summaries. |
| .github/skills/run-tests/SKILL.md | Documents runner usage. |
Review effort: Balanced
Comment on lines
+259
to
+260
| string candidate = argument.Trim('"'); | ||
| string candidatePath = TestPathUtility.ResolveFullPath(candidate, workingDirectory); |
| { | ||
| testBinlogCapture.Complete(result.ExitCode); | ||
| } | ||
| else if (Environment.GetEnvironmentVariable("HELIX_WORKITEM_UPLOAD_ROOT") is string uploadRoot) |
Member
Author
There was a problem hiding this comment.
Given that this skill is really meant for 1-off investigations, there won't be an associated Helix run to capture these logs. So this change isn't needed.
| return Path.GetFullPath(Path.Combine(Path.GetPathRoot(basePath)!, path.TrimStart('\\', '/'))); | ||
| } | ||
|
|
||
| return Path.GetFullPath(Path.IsPathRooted(path) ? path : Path.Combine(basePath, path)); |
Comment on lines
+111
to
+112
| commandSpec.Arguments.Add($"/bl:{binlogPath}"); | ||
| destinations.Add(new LoggerDestination(binlogPath, true, false)); |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c8b91f0-8d92-4828-a52c-1fc9e63fe773
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c8b91f0-8d92-4828-a52c-1fc9e63fe773
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The test runner's
build.binlogcaptures the test project build, not necessarily builds performed by tests. When those scenario builds fail, their logs may not be available for diagnosis unless tests are changed to request binlogs.Description
Add opt-in
--test-binlogssupport to capture build logs fromMSBuildCommandandDotnetBuildCommand. Retain logs in a per-runtest-binlogsdirectory, preserve explicitly configured logger destinations, and include the directory and binlog count in runner output. Capture is disabled unless requested.Fixes #56658