Repository navigation
Preserve complete CTRF attachment paths - #12308
Conversation
Keep attachment references intact instead of appending diagnostic truncation text. Retain descriptive metadata limits and cover boundaries, retry output, and a real packaged runner. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🧪 Test quality review — PR #12308
Compared merge base 8bdfc29f415696738d845d56aca5d438d0a068de with head e3c4c54e15c3f183f4fcefaa971b467a4b562961. Evidence applies to this head, not every intermediate commit.
Test-value evidence
All results below are static predictions. No tests were executed and no paired red/green records were available.
| Contract | Scenario and expected outcome | Test and decisive assertion | Counterfactual | Correct implementation |
|---|---|---|---|---|
Attachment path is no longer truncated |
A path longer than MaxIdentityFieldLength is emitted unchanged, in both the final and retry-attempt attachments. |
TestResultCapture_PreservesAttachmentPathInFinalAndRetryReports (rows -1/0/1) asserts path equals the input at the capture and report levels. Ctrf_LongAttachmentPath_IsPreserved asserts the same end to end with a path over 4096 characters. |
Restoring Truncate(fullPath, MaxIdentityFieldLength) would predictably fail the +0/+1 rows and the acceptance test. |
Predicted to pass. |
| Attachment name and description keep their limits | Metadata is still truncated without splitting surrogate pairs. | TestResultCapture_TruncatesAttachmentMetadataWithoutSplittingSurrogatePairs (AssertTruncated on name and description). |
Removing name truncation would fail it. | Predicted to pass. |
PACKAGE.md wording |
Documentation only. | n/a | n/a | Not applicable. |
🧵 Parallel safety
STATUS: PARTIAL. The specialist analysis was not completed in this run, so this is not a clean result. Static reading of the changed tests found nothing concerning.
- The acceptance test passes its path through a per-process environment variable, and its results directory is a unique GUID.
- The fixture's default
failure.logwrite is untouched when the variable is unset.
Per-test assessment
Pseudo-mutation evidence is a static prediction, not an executed mutation result.
| Result | Quality | Test | Notes | How to improve |
|---|---|---|---|---|
| Pass | A (90–100) | new CtrfReportTests.Ctrf_LongAttachmentPath_IsPreserved |
Asserts exact path, name, status and count end to end. | None |
| Pass | A (90–100) | new CtrfReportEngineTests.TestResultCapture_PreservesAttachmentPathInFinalAndRetryReports |
Boundary rows -1/0/1 check the exact path in the final and retry attempts. | None |
| Pass | A (90–100) | mod CtrfReportEngineTests.TestResultCapture_TruncatesAttachmentMetadataWithoutSplittingSurrogatePairs |
Exact path equality plus name and description truncation checks. | None |
No inline findings were published.
<sub>This advisory review is heuristic and non-blocking.
Re-run the combined review with /review-tests, or only the parallel-safety specialist with /parallel-audit.</sub>
🤖 Automated content by GitHub Copilot. Generated by the Test Quality Review on PR (on open / sync) workflow. · copilot · auto · ~20 AIC · ⌖ ~1 AIC · ⊞ 8.1K
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
Result: no confirmed findings.
Scope reviewed: TestResultCapture.cs (attachment Path no longer truncated), the PACKAGE.md wording, the unit tests (boundary lengths −1/0/+1 through final and retry reports) and the acceptance test (path over 4096 chars with & and a surrogate pair).
- Behavior:
attachment.Pathis the only truncated field removed.NameandDescriptionkeep their limits. The path is written verbatim inCtrfReportEngine.JsonTestWriter.cs(WriteString("path", ...)), so JSON escaping covers special characters. I found no other consumer that depends on the old truncation. - Compatibility: This is a report-content change only. There is no public API or IPC change, and no new resources or localization.
- Tests: The tests assert exact path equality at the old limit boundary and across retry merging. They use the MSTest style these projects require. The old surrogate-pair truncation case now only applies to name and description.
- Test-value evidence: Not executed (read-only review). By static prediction, the new tests would fail on the base revision because
Pathwas truncated toMaxIdentityFieldLength. This is a prediction, not a measured result. - Evidence gap: I did not run the over-4096-character path acceptance test on Windows or macOS, where path-length handling differs. It sets an environment variable and never creates the file, so it relies on
GetFullPathaccepting long paths. CI should confirm this.
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the attachment-path preservation change.
No open issues 🎉
What changed in this PR
Fixes Microsoft.Testing.Platform’s CTRF reporter so attachment paths longer than 4,096 characters remain complete, while attachment names and descriptions retain their existing size limits.
Changes:
- Removes truncation from captured attachment paths.
- Adds boundary, Unicode, retry-report, and package-consuming regression coverage.
- Clarifies the distinction between exact paths and bounded diagnostic metadata.
Test-value evidence: The PR reports pre-fix failures and passing fixed runs for revision e3c4c54e15c3f183f4fcefaa971b467a4b562961. Execution artifacts were not independently verified in this review.
| File | Description |
|---|---|
| test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CtrfReportEngineTests.cs | Checks exact paths at length boundaries and in final/retry reports; retains metadata truncation checks. |
| test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CtrfReportTests.cs | Verifies a long Unicode path in packaged output on the correct failed result. |
| src/Platform/Microsoft.Testing.Extensions.CtrfReport/TestResultCapture.cs | Preserves the complete attachment path. |
| src/Platform/Microsoft.Testing.Extensions.CtrfReport/PACKAGE.md | Documents untruncated paths and bounded names and descriptions. |
Review effort: Balanced
Pull request
Summary
CTRF attachment paths longer than 4,096 characters were shortened and given a diagnostic truncation suffix, making the reported reference unusable. Preserve the complete path while retaining the existing size limits for attachment names and descriptions.
Relates to ctrf-io/ctrf#68.
Design and decision record
This is a focused correctness fix, not a new feature, so no separate decision document is needed. The upstream attachment guidance treats
pathas an opaque reference and permits local paths. A locator cannot use the same lossy truncation as descriptive text.Only the captured
Pathassignment changes. No API, schema, CLI, upload, digest, or rerun-metadata changes are included. The package documentation now explicitly distinguishes exact references from bounded diagnostic metadata.Demonstration
Ctrf_LongAttachmentPath_IsPreservedexercises a real package-consuming runner with a long Unicode reference attached specifically to failed resulttest-2/FailingTest.This is a machine-readable reference-fidelity change, not a rendered layout change; screenshots are inapplicable. The metadata-only attachment does not claim that a file exists or was uploaded.
Validation and regression proof
Validated on Windows x64, Debug, with the repository-local SDK
12.0.100-alpha.1.26556.115. Tested source is now commite3c4c54e15c3f183f4fcefaa971b467a4b562961, based on8bdfc29f415696738d845d56aca5d438d0a068de; the committed files match the previously tested patch byte-for-byte.net10.0,net11.0,net462,net472CtrfReportEngineTestsclass onnet11.0Ctrf_LongAttachmentPath_IsPreserved, outernet12.0, packaged childnet11.0net11.0,net462) and both mixed-version merge input ordersBoth production states were rebuilt and packed separately. Separate task-owned outputs and private acceptance caches prevented stale-package reuse; the regression test sources were identical in both states. The cached and executed reporter DLLs matched each corresponding package payload.
Executed commands and retained evidence
Commands ran from the worktree root.
$evidencedenotes the retained session artifact directory,$resultsthe distinct directory for each run, and$binloga unique file within$evidence. Log redirection is omitted below.Final builds and pack: all exited 0 with no warnings or errors.
The unit build covers all four test TFMs; the package covers
netstandard2.0,net10.0, andnet11.0. The existing acceptance fixture's prerequisite packages were also packed successfully, including the complete MSTest product-set validation.Focused regression invocations: 4 passed per target, exit 0.
Each invocation used its own
$resultsdirectory and corresponding report arguments. The pre-fix comparison used the same invocations and assertions with modules underartifacts\bin\reference-red: each target executed four cases, passed two, failed the two intended over-limit cases, skipped none, and exited 2.Other executed selections: 70, 1, and 4 passed respectively; each exited 0.
Acceptance runs selected distinct red/green marked caches through
TESTFX_COLLECTION_NUGET_CACHE. Each process used the worktree's.dotnetthroughDOTNET_ROOT,DOTNET_ROOT_X64, andDOTNET_MULTILEVEL_LOOKUP=0. Actual package, cache, and consumer DLL hashes were compared rather than assuming a same-version restore was fresh.Independent discovery and expected case lists were reconciled with executed TRX records. Each green selection passed:
Actual TRX, CTRF, and JUnit exports were parsed and their case counts reconciled.
Retained local evidence:
reference-fidelity/validation-evidence.json, per-run TRX/CTRF/JUnit and execution logs, package provenance, patches, and unique build binlogs. These artifacts are attached to the Copilot session, not public GitHub artifact links; the decisive results are included above. Temporary comparison outputs, private caches, and generated assets were removed.304DCE4E7F3FC1EEB9C250950DFD444D87A08E413D7ACA96DC95786C4E1CCF896D7AA092C7A6C33AB681921FBDE061303A97DDEC6F16E979BF9178AD0C7CBDADMicrosoft.Testing.Extensions.CtrfReport.3.0.0-dev.nupkg4F9291DD74145B9B371C0E8DA9ED8BBA593DF148835A0C9E13FFB88A5166DC37net11.0DLL34A46C732A3B380A8A23298FEFFB08002B63B02E0B1E37BDA28E452B9CCC62CBnet11.0DLL7157723BD68651410B9196FA94212F611D812BF5C3F3A76CD37DA20A2722E8B8Resolved setup/packaging failures are retained separately and are not counted as behavioral red evidence. No full solution test run, non-Windows execution, release-branch validation, or CI result is claimed.
Test-value evidence
Counterfactual: actual pre-fix capture code at the base revision, retaining the exact new tests and assertions. Both states compiled and executed; setup failures were excluded.
TestResultCapture_PreservesAttachmentPathInFinalAndRetryReports (-1),(0),(1)(1)fails exact-path equality; boundary rows pass on all four TFMsTestResultCapture_TruncatesAttachmentMetadataWithoutSplittingSurrogatePairsCtrf_LongAttachmentPath_IsPreserved