Fix Dependabot discovery of bundled Playwright via PackageDownload - #9452

Merged
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp
Jun 26, 2026
Merged

Fix Dependabot discovery of bundled Playwright via PackageDownload#9452
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Problem

Microsoft.Playwright.MSTest.v4 is still stuck at 1.60.0 even though 1.61.0 has been on the feeds since 2026-06-24 and the anchor fix#9422 has merged. Dependabot keeps updating Aspire.Hosting.Testing but never bumps Playwright (#9362).

Root cause: the #9422 anchor mechanism doesn't work

#9422 added inert anchors as PackageReference ... ExcludeAssets="all". But ExcludeAssets="all" removes the reference from the dependency graph that Dependabot reads, so the package stays invisible to Dependabot and is never proposed for an update — the opposite of the anchor's intent.

The in-repo evidence is a clean A/B from today's Dependabot run (06:47 UTC — after #9422 landed, after 1.61.0 shipped, with 0 open Dependabot PRs so the limit of 15 is irrelevant):

On mainReferenceResult
Aspire.Hosting.TestingPackageDownload+ExcludeAssets="all" anchor✅ bumped 13.2.1 → 13.4.6
Microsoft.Playwright.MSTest.v4ExcludeAssets="all" anchor only❌ stuck at 1.60.0

Same run, same config, same feeds. The only difference is the PackageDownload: Aspire updates via that, not via the ExcludeAssets anchor. Playwright, having only the anchor, is invisible.

Fix

  • Replace the inert ExcludeAssets="all"PackageReference anchors with PackageDownload items for both packages. PackageDownload keeps the package in the restore graph (so Dependabot proposes bumps) without flowing any of its assets (compile/runtime/build — e.g. Playwright's browser install) into the test project. This is the mechanism already proven by Aspire.
  • Correct the now-misleading "condition 2" guidance in Directory.Packages.props and the anchor comment in the acceptance csproj to document that the anchor must be a PackageDownload, not a PackageReference with ExcludeAssets="all".

The _ValidateBundledSdkFeatureVersions drift guard is unchanged and still keeps MicrosoftPlaywrightVersion / AspireHostingTestingVersion in sync with the literal PackageVersion entries.

Validation

MSTest.Acceptance.IntegrationTests restores and builds cleanly (0 warnings, 0 errors).

Notes

This effectively re-applies the PackageDownload approach from the earlier #9449 (which I had closed as "superseded" by #9422 — but #9422's anchor turned out not to work). Supersedes the Playwright-relevant part of #9422.

Closes#9362.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

The orphan `PackageVersion` for `Microsoft.Playwright.MSTest.v4` in
Directory.Packages.props was never enough for Dependabot to discover and
bump it: Dependabot only proposes updates for packages that have a real
consuming reference (PackageReference / PackageDownload). Aspire.Hosting.Testing
sits in the same orphan-CPM ItemGroup but was kept up to date only because it
already had a PackageDownload in the acceptance-test project (see #9446, which
bumped Aspire but left Playwright stuck at 1.60.0).
Add a matching PackageDownload for Microsoft.Playwright.MSTest.v4 and update the
surrounding comments so future bundled SDK feature packages follow the same
pattern.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
main independently fixed#9362 via inert PackageReference ExcludeAssets=\"all\"
anchors for both Playwright and Aspire. Revert this branch's PackageDownload-based
approach and the contradictory comments to match main exactly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The anchor added in #9422 used a PackageReference with ExcludeAssets=\"all\",
which removes the reference from the dependency graph Dependabot reads, so the
package stays invisible and is never bumped. Aspire.Hosting.Testing kept updating
only because it ALSO has a PackageDownload; Microsoft.Playwright.MSTest.v4 had only
the ExcludeAssets anchor and stayed stuck at 1.60.0 (today's Dependabot run bumped
Aspire to 13.4.6 but skipped Playwright, with no PR-limit pressure).
Replace the inert ExcludeAssets anchors with PackageDownload (proven by Aspire),
which keeps the package in the restore graph without flowing any of its assets into
the test project, and correct the now-misleading guidance comments.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 26, 2026 10:57

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the repo’s “Dependabot anchor” mechanism for SDK-bundled feature packages (Playwright + Aspire) so Dependabot can consistently discover and propose version bumps without flowing package assets into the acceptance test project.

Changes:

  • Replaced ExcludeAssets="all"PackageReference anchors with PackageDownload anchors for Aspire.Hosting.Testing and Microsoft.Playwright.MSTest.v4.
  • Consolidated and expanded documentation in the acceptance csproj explaining why PackageDownload is required for Dependabot discovery.
  • Updated Directory.Packages.props guidance to reflect the PackageDownload-based anchoring approach.
Show a summary per file
FileDescription
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csprojSwaps Dependabot anchors to PackageDownload for both bundled feature packages and updates the explanatory comment.
Directory.Packages.propsUpdates documentation explaining Dependabot prerequisites and clarifies that anchors must be PackageDownload.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment threadDirectory.Packages.props Outdated
@Evangelink

This comment has been minimized.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review: PR #9452 — Fix Dependabot discovery of bundled Playwright via PackageDownload

#DimensionSeverityResult
1Algorithmic CorrectnessMAJOR✅ N/A — no algorithmic changes
2Threading & ConcurrencyBLOCKING✅ N/A — no C# code
3SecurityBLOCKING✅ No concerns — only NuGet infrastructure touching well-known packages
4Error HandlingMAJOR_ValidateBundledSdkFeatureVersions guard intact and still fires after a Dependabot bump
5Nullability / Null SafetyMAJOR✅ N/A
6Resource ManagementMAJOR✅ N/A
7API Design & ContractsMAJOR✅ N/A — no public API surface touched
8Test CoverageMINORi️ No automated test can exercise Dependabot's bump behavior; the fix relies on empirical evidence (Aspire kept updating because it had a PackageDownload; Playwright did not because it only had ExcludeAssets="all") — acceptable given the nature of the change
9PerformanceMINORPackageDownload downloads the nupkg to the global cache during restore — marginally more work than the old PackageReference ExcludeAssets="all", but negligible and intentional
10Logging & ObservabilityMINOR✅ N/A
11Configuration & CompatibilityMAJORPackageDownload always requires an explicit version even under CPM — [$(MicrosoftPlaywrightVersion)] and [$(AspireHostingTestingVersion)] are correct: MSBuild evaluates the property first, so NuGet sees [1.60.0] / [13.4.6], both valid exact-version bracket constraints
12Code Clarity & MaintainabilityMINOR✅ Consolidating two separate ItemGroup blocks into one with a unified, well-explained comment is cleaner than the previous split
13Documentation AccuracyMINORi️ See inline comment on line 46 of the .csproj: "also used to stage nupkgs for test assets" is accurate only for the Aspire entry; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Also, both files use the phrase "restore graph" for PackageDownload items — technically these appear under packageDownloads in project.assets.json, not in the dependency/restore graph proper; functionally accurate for Dependabot purposes
14Dependency ManagementMAJOR✅ Core fix is correct. Empirical evidence: Aspire was getting Dependabot bumps (it had a PackageDownload); Playwright was not (it only had PackageReference ExcludeAssets="all"). Switching Playwright to PackageDownload aligns both packages on the working pattern. The Aspire duplicate PackageReference ExcludeAssets="all" is correctly removed as it was redundant
15MSBuild / NuGet CorrectnessMAJORPackageDownload item type, bracket-notation exact-version requirement, and MSBuild property expansion all used correctly. test/Directory.Build.targets adds GeneratePathProperty="True" for Microsoft.Testing.Extensions.CodeCoverage, so $(PkgMicrosoft_Testing_Extensions_CodeCoverage) in CopyNuGetPackagesForTestAssets is unaffected by this change
16Public API SurfaceBLOCKING✅ N/A — build infrastructure only
17CI / CD PipelineMAJOR✅ Dependabot config (directory: "/") scans the entire repo; MSTest.Acceptance.IntegrationTests.csproj is picked up. After this change both Aspire and Playwright entries should receive daily bump proposals
18Layering & ArchitectureMINOR✅ Correct layer to host Dependabot anchors — the acceptance test project participates in the full NuGet restore and is already the canonical home for such anchors (see #9362)
19Dead CodeMINOR✅ The ineffective PackageReference ExcludeAssets="all" anchors are cleanly removed for both packages — no dead items remain
20Breaking ChangesBLOCKING✅ N/A — internal build plumbing; no shipped API surface changes
21Cross-PlatformMINORPackageDownload is NuGet-native and platform-agnostic
22Code Style & FormattingMINOR✅ Alphabetical ordering (Aspire before Playwright) is consistent with existing conventions; comment indentation and line-wrapping match surrounding style

Overall Assessment

The root-cause diagnosis is accurate and the fix is minimal and correct. The decision to replace PackageReference ... ExcludeAssets="all" with PackageDownload is well-supported by empirical evidence and by how Dependabot processes NuGet manifests. All supporting machinery (_ValidateBundledSdkFeatureVersions, CopyNuGetPackagesForTestAssets, CPM version literals) continues to work correctly without modification.

One non-blocking inline comment is filed: the parenthetical "also used to stage nupkgs for test assets" in the .csproj comment applies to the Aspire entry only; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Worth tightening in a follow-up if desired, but not a blocker.

- Note that the bundled packages ARE referenced inside the SDK feature targets
(Sdk/Features/*.targets); what's missing is a graph-visible reference from a
restored repo project, which is what Dependabot relies on.
- Clarify that only the Aspire PackageDownload stages a nupkg for test assets;
the Playwright PackageDownload is the Dependabot anchor only.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

🧪 Test quality grade — PR #9452

No new or modified test methods were identified in the changed regions
of this PR. Nothing to grade.

Re-run with /grade-tests.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 90.5 AIC · ⌖ 12.9 AIC · ⊞ 43.8K · [◷]( · )

@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jun 26, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update bundled Playwright for .NET version in MSTest SDK

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Fix Dependabot discovery of bundled Playwright via PackageDownload - #9452

Merged
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp
Jun 26, 2026
Merged

Fix Dependabot discovery of bundled Playwright via PackageDownload#9452
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Problem

Microsoft.Playwright.MSTest.v4 is still stuck at 1.60.0 even though 1.61.0 has been on the feeds since 2026-06-24 and the anchor fix#9422 has merged. Dependabot keeps updating Aspire.Hosting.Testing but never bumps Playwright (#9362).

Root cause: the #9422 anchor mechanism doesn't work

#9422 added inert anchors as PackageReference ... ExcludeAssets="all". But ExcludeAssets="all" removes the reference from the dependency graph that Dependabot reads, so the package stays invisible to Dependabot and is never proposed for an update — the opposite of the anchor's intent.

The in-repo evidence is a clean A/B from today's Dependabot run (06:47 UTC — after #9422 landed, after 1.61.0 shipped, with 0 open Dependabot PRs so the limit of 15 is irrelevant):

On mainReferenceResult
Aspire.Hosting.TestingPackageDownload+ExcludeAssets="all" anchor✅ bumped 13.2.1 → 13.4.6
Microsoft.Playwright.MSTest.v4ExcludeAssets="all" anchor only❌ stuck at 1.60.0

Same run, same config, same feeds. The only difference is the PackageDownload: Aspire updates via that, not via the ExcludeAssets anchor. Playwright, having only the anchor, is invisible.

Fix

  • Replace the inert ExcludeAssets="all"PackageReference anchors with PackageDownload items for both packages. PackageDownload keeps the package in the restore graph (so Dependabot proposes bumps) without flowing any of its assets (compile/runtime/build — e.g. Playwright's browser install) into the test project. This is the mechanism already proven by Aspire.
  • Correct the now-misleading "condition 2" guidance in Directory.Packages.props and the anchor comment in the acceptance csproj to document that the anchor must be a PackageDownload, not a PackageReference with ExcludeAssets="all".

The _ValidateBundledSdkFeatureVersions drift guard is unchanged and still keeps MicrosoftPlaywrightVersion / AspireHostingTestingVersion in sync with the literal PackageVersion entries.

Validation

MSTest.Acceptance.IntegrationTests restores and builds cleanly (0 warnings, 0 errors).

Notes

This effectively re-applies the PackageDownload approach from the earlier #9449 (which I had closed as "superseded" by #9422 — but #9422's anchor turned out not to work). Supersedes the Playwright-relevant part of #9422.

Closes#9362.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

The orphan `PackageVersion` for `Microsoft.Playwright.MSTest.v4` in
Directory.Packages.props was never enough for Dependabot to discover and
bump it: Dependabot only proposes updates for packages that have a real
consuming reference (PackageReference / PackageDownload). Aspire.Hosting.Testing
sits in the same orphan-CPM ItemGroup but was kept up to date only because it
already had a PackageDownload in the acceptance-test project (see #9446, which
bumped Aspire but left Playwright stuck at 1.60.0).
Add a matching PackageDownload for Microsoft.Playwright.MSTest.v4 and update the
surrounding comments so future bundled SDK feature packages follow the same
pattern.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
main independently fixed#9362 via inert PackageReference ExcludeAssets=\"all\"
anchors for both Playwright and Aspire. Revert this branch's PackageDownload-based
approach and the contradictory comments to match main exactly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The anchor added in #9422 used a PackageReference with ExcludeAssets=\"all\",
which removes the reference from the dependency graph Dependabot reads, so the
package stays invisible and is never bumped. Aspire.Hosting.Testing kept updating
only because it ALSO has a PackageDownload; Microsoft.Playwright.MSTest.v4 had only
the ExcludeAssets anchor and stayed stuck at 1.60.0 (today's Dependabot run bumped
Aspire to 13.4.6 but skipped Playwright, with no PR-limit pressure).
Replace the inert ExcludeAssets anchors with PackageDownload (proven by Aspire),
which keeps the package in the restore graph without flowing any of its assets into
the test project, and correct the now-misleading guidance comments.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 26, 2026 10:57

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the repo’s “Dependabot anchor” mechanism for SDK-bundled feature packages (Playwright + Aspire) so Dependabot can consistently discover and propose version bumps without flowing package assets into the acceptance test project.

Changes:

  • Replaced ExcludeAssets="all"PackageReference anchors with PackageDownload anchors for Aspire.Hosting.Testing and Microsoft.Playwright.MSTest.v4.
  • Consolidated and expanded documentation in the acceptance csproj explaining why PackageDownload is required for Dependabot discovery.
  • Updated Directory.Packages.props guidance to reflect the PackageDownload-based anchoring approach.
Show a summary per file
FileDescription
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csprojSwaps Dependabot anchors to PackageDownload for both bundled feature packages and updates the explanatory comment.
Directory.Packages.propsUpdates documentation explaining Dependabot prerequisites and clarifies that anchors must be PackageDownload.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment threadDirectory.Packages.props Outdated
@Evangelink

This comment has been minimized.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review: PR #9452 — Fix Dependabot discovery of bundled Playwright via PackageDownload

#DimensionSeverityResult
1Algorithmic CorrectnessMAJOR✅ N/A — no algorithmic changes
2Threading & ConcurrencyBLOCKING✅ N/A — no C# code
3SecurityBLOCKING✅ No concerns — only NuGet infrastructure touching well-known packages
4Error HandlingMAJOR_ValidateBundledSdkFeatureVersions guard intact and still fires after a Dependabot bump
5Nullability / Null SafetyMAJOR✅ N/A
6Resource ManagementMAJOR✅ N/A
7API Design & ContractsMAJOR✅ N/A — no public API surface touched
8Test CoverageMINORi️ No automated test can exercise Dependabot's bump behavior; the fix relies on empirical evidence (Aspire kept updating because it had a PackageDownload; Playwright did not because it only had ExcludeAssets="all") — acceptable given the nature of the change
9PerformanceMINORPackageDownload downloads the nupkg to the global cache during restore — marginally more work than the old PackageReference ExcludeAssets="all", but negligible and intentional
10Logging & ObservabilityMINOR✅ N/A
11Configuration & CompatibilityMAJORPackageDownload always requires an explicit version even under CPM — [$(MicrosoftPlaywrightVersion)] and [$(AspireHostingTestingVersion)] are correct: MSBuild evaluates the property first, so NuGet sees [1.60.0] / [13.4.6], both valid exact-version bracket constraints
12Code Clarity & MaintainabilityMINOR✅ Consolidating two separate ItemGroup blocks into one with a unified, well-explained comment is cleaner than the previous split
13Documentation AccuracyMINORi️ See inline comment on line 46 of the .csproj: "also used to stage nupkgs for test assets" is accurate only for the Aspire entry; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Also, both files use the phrase "restore graph" for PackageDownload items — technically these appear under packageDownloads in project.assets.json, not in the dependency/restore graph proper; functionally accurate for Dependabot purposes
14Dependency ManagementMAJOR✅ Core fix is correct. Empirical evidence: Aspire was getting Dependabot bumps (it had a PackageDownload); Playwright was not (it only had PackageReference ExcludeAssets="all"). Switching Playwright to PackageDownload aligns both packages on the working pattern. The Aspire duplicate PackageReference ExcludeAssets="all" is correctly removed as it was redundant
15MSBuild / NuGet CorrectnessMAJORPackageDownload item type, bracket-notation exact-version requirement, and MSBuild property expansion all used correctly. test/Directory.Build.targets adds GeneratePathProperty="True" for Microsoft.Testing.Extensions.CodeCoverage, so $(PkgMicrosoft_Testing_Extensions_CodeCoverage) in CopyNuGetPackagesForTestAssets is unaffected by this change
16Public API SurfaceBLOCKING✅ N/A — build infrastructure only
17CI / CD PipelineMAJOR✅ Dependabot config (directory: "/") scans the entire repo; MSTest.Acceptance.IntegrationTests.csproj is picked up. After this change both Aspire and Playwright entries should receive daily bump proposals
18Layering & ArchitectureMINOR✅ Correct layer to host Dependabot anchors — the acceptance test project participates in the full NuGet restore and is already the canonical home for such anchors (see #9362)
19Dead CodeMINOR✅ The ineffective PackageReference ExcludeAssets="all" anchors are cleanly removed for both packages — no dead items remain
20Breaking ChangesBLOCKING✅ N/A — internal build plumbing; no shipped API surface changes
21Cross-PlatformMINORPackageDownload is NuGet-native and platform-agnostic
22Code Style & FormattingMINOR✅ Alphabetical ordering (Aspire before Playwright) is consistent with existing conventions; comment indentation and line-wrapping match surrounding style

Overall Assessment

The root-cause diagnosis is accurate and the fix is minimal and correct. The decision to replace PackageReference ... ExcludeAssets="all" with PackageDownload is well-supported by empirical evidence and by how Dependabot processes NuGet manifests. All supporting machinery (_ValidateBundledSdkFeatureVersions, CopyNuGetPackagesForTestAssets, CPM version literals) continues to work correctly without modification.

One non-blocking inline comment is filed: the parenthetical "also used to stage nupkgs for test assets" in the .csproj comment applies to the Aspire entry only; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Worth tightening in a follow-up if desired, but not a blocker.

- Note that the bundled packages ARE referenced inside the SDK feature targets
(Sdk/Features/*.targets); what's missing is a graph-visible reference from a
restored repo project, which is what Dependabot relies on.
- Clarify that only the Aspire PackageDownload stages a nupkg for test assets;
the Playwright PackageDownload is the Dependabot anchor only.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

🧪 Test quality grade — PR #9452

No new or modified test methods were identified in the changed regions
of this PR. Nothing to grade.

Re-run with /grade-tests.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 90.5 AIC · ⌖ 12.9 AIC · ⊞ 43.8K · [◷]( · )

@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jun 26, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update bundled Playwright for .NET version in MSTest SDK

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix Dependabot discovery of bundled Playwright via PackageDownload - #9452

Merged
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp
Jun 26, 2026
Merged

Fix Dependabot discovery of bundled Playwright via PackageDownload#9452
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Problem

Microsoft.Playwright.MSTest.v4 is still stuck at 1.60.0 even though 1.61.0 has been on the feeds since 2026-06-24 and the anchor fix#9422 has merged. Dependabot keeps updating Aspire.Hosting.Testing but never bumps Playwright (#9362).

Root cause: the #9422 anchor mechanism doesn't work

#9422 added inert anchors as PackageReference ... ExcludeAssets="all". But ExcludeAssets="all" removes the reference from the dependency graph that Dependabot reads, so the package stays invisible to Dependabot and is never proposed for an update — the opposite of the anchor's intent.

The in-repo evidence is a clean A/B from today's Dependabot run (06:47 UTC — after #9422 landed, after 1.61.0 shipped, with 0 open Dependabot PRs so the limit of 15 is irrelevant):

On mainReferenceResult
Aspire.Hosting.TestingPackageDownload+ExcludeAssets="all" anchor✅ bumped 13.2.1 → 13.4.6
Microsoft.Playwright.MSTest.v4ExcludeAssets="all" anchor only❌ stuck at 1.60.0

Same run, same config, same feeds. The only difference is the PackageDownload: Aspire updates via that, not via the ExcludeAssets anchor. Playwright, having only the anchor, is invisible.

Fix

  • Replace the inert ExcludeAssets="all"PackageReference anchors with PackageDownload items for both packages. PackageDownload keeps the package in the restore graph (so Dependabot proposes bumps) without flowing any of its assets (compile/runtime/build — e.g. Playwright's browser install) into the test project. This is the mechanism already proven by Aspire.
  • Correct the now-misleading "condition 2" guidance in Directory.Packages.props and the anchor comment in the acceptance csproj to document that the anchor must be a PackageDownload, not a PackageReference with ExcludeAssets="all".

The _ValidateBundledSdkFeatureVersions drift guard is unchanged and still keeps MicrosoftPlaywrightVersion / AspireHostingTestingVersion in sync with the literal PackageVersion entries.

Validation

MSTest.Acceptance.IntegrationTests restores and builds cleanly (0 warnings, 0 errors).

Notes

This effectively re-applies the PackageDownload approach from the earlier #9449 (which I had closed as "superseded" by #9422 — but #9422's anchor turned out not to work). Supersedes the Playwright-relevant part of #9422.

Closes#9362.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

The orphan `PackageVersion` for `Microsoft.Playwright.MSTest.v4` in
Directory.Packages.props was never enough for Dependabot to discover and
bump it: Dependabot only proposes updates for packages that have a real
consuming reference (PackageReference / PackageDownload). Aspire.Hosting.Testing
sits in the same orphan-CPM ItemGroup but was kept up to date only because it
already had a PackageDownload in the acceptance-test project (see #9446, which
bumped Aspire but left Playwright stuck at 1.60.0).
Add a matching PackageDownload for Microsoft.Playwright.MSTest.v4 and update the
surrounding comments so future bundled SDK feature packages follow the same
pattern.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
main independently fixed#9362 via inert PackageReference ExcludeAssets=\"all\"
anchors for both Playwright and Aspire. Revert this branch's PackageDownload-based
approach and the contradictory comments to match main exactly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The anchor added in #9422 used a PackageReference with ExcludeAssets=\"all\",
which removes the reference from the dependency graph Dependabot reads, so the
package stays invisible and is never bumped. Aspire.Hosting.Testing kept updating
only because it ALSO has a PackageDownload; Microsoft.Playwright.MSTest.v4 had only
the ExcludeAssets anchor and stayed stuck at 1.60.0 (today's Dependabot run bumped
Aspire to 13.4.6 but skipped Playwright, with no PR-limit pressure).
Replace the inert ExcludeAssets anchors with PackageDownload (proven by Aspire),
which keeps the package in the restore graph without flowing any of its assets into
the test project, and correct the now-misleading guidance comments.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 26, 2026 10:57

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the repo’s “Dependabot anchor” mechanism for SDK-bundled feature packages (Playwright + Aspire) so Dependabot can consistently discover and propose version bumps without flowing package assets into the acceptance test project.

Changes:

  • Replaced ExcludeAssets="all"PackageReference anchors with PackageDownload anchors for Aspire.Hosting.Testing and Microsoft.Playwright.MSTest.v4.
  • Consolidated and expanded documentation in the acceptance csproj explaining why PackageDownload is required for Dependabot discovery.
  • Updated Directory.Packages.props guidance to reflect the PackageDownload-based anchoring approach.
Show a summary per file
FileDescription
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csprojSwaps Dependabot anchors to PackageDownload for both bundled feature packages and updates the explanatory comment.
Directory.Packages.propsUpdates documentation explaining Dependabot prerequisites and clarifies that anchors must be PackageDownload.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment threadDirectory.Packages.props Outdated
@Evangelink

This comment has been minimized.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review: PR #9452 — Fix Dependabot discovery of bundled Playwright via PackageDownload

#DimensionSeverityResult
1Algorithmic CorrectnessMAJOR✅ N/A — no algorithmic changes
2Threading & ConcurrencyBLOCKING✅ N/A — no C# code
3SecurityBLOCKING✅ No concerns — only NuGet infrastructure touching well-known packages
4Error HandlingMAJOR_ValidateBundledSdkFeatureVersions guard intact and still fires after a Dependabot bump
5Nullability / Null SafetyMAJOR✅ N/A
6Resource ManagementMAJOR✅ N/A
7API Design & ContractsMAJOR✅ N/A — no public API surface touched
8Test CoverageMINORi️ No automated test can exercise Dependabot's bump behavior; the fix relies on empirical evidence (Aspire kept updating because it had a PackageDownload; Playwright did not because it only had ExcludeAssets="all") — acceptable given the nature of the change
9PerformanceMINORPackageDownload downloads the nupkg to the global cache during restore — marginally more work than the old PackageReference ExcludeAssets="all", but negligible and intentional
10Logging & ObservabilityMINOR✅ N/A
11Configuration & CompatibilityMAJORPackageDownload always requires an explicit version even under CPM — [$(MicrosoftPlaywrightVersion)] and [$(AspireHostingTestingVersion)] are correct: MSBuild evaluates the property first, so NuGet sees [1.60.0] / [13.4.6], both valid exact-version bracket constraints
12Code Clarity & MaintainabilityMINOR✅ Consolidating two separate ItemGroup blocks into one with a unified, well-explained comment is cleaner than the previous split
13Documentation AccuracyMINORi️ See inline comment on line 46 of the .csproj: "also used to stage nupkgs for test assets" is accurate only for the Aspire entry; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Also, both files use the phrase "restore graph" for PackageDownload items — technically these appear under packageDownloads in project.assets.json, not in the dependency/restore graph proper; functionally accurate for Dependabot purposes
14Dependency ManagementMAJOR✅ Core fix is correct. Empirical evidence: Aspire was getting Dependabot bumps (it had a PackageDownload); Playwright was not (it only had PackageReference ExcludeAssets="all"). Switching Playwright to PackageDownload aligns both packages on the working pattern. The Aspire duplicate PackageReference ExcludeAssets="all" is correctly removed as it was redundant
15MSBuild / NuGet CorrectnessMAJORPackageDownload item type, bracket-notation exact-version requirement, and MSBuild property expansion all used correctly. test/Directory.Build.targets adds GeneratePathProperty="True" for Microsoft.Testing.Extensions.CodeCoverage, so $(PkgMicrosoft_Testing_Extensions_CodeCoverage) in CopyNuGetPackagesForTestAssets is unaffected by this change
16Public API SurfaceBLOCKING✅ N/A — build infrastructure only
17CI / CD PipelineMAJOR✅ Dependabot config (directory: "/") scans the entire repo; MSTest.Acceptance.IntegrationTests.csproj is picked up. After this change both Aspire and Playwright entries should receive daily bump proposals
18Layering & ArchitectureMINOR✅ Correct layer to host Dependabot anchors — the acceptance test project participates in the full NuGet restore and is already the canonical home for such anchors (see #9362)
19Dead CodeMINOR✅ The ineffective PackageReference ExcludeAssets="all" anchors are cleanly removed for both packages — no dead items remain
20Breaking ChangesBLOCKING✅ N/A — internal build plumbing; no shipped API surface changes
21Cross-PlatformMINORPackageDownload is NuGet-native and platform-agnostic
22Code Style & FormattingMINOR✅ Alphabetical ordering (Aspire before Playwright) is consistent with existing conventions; comment indentation and line-wrapping match surrounding style

Overall Assessment

The root-cause diagnosis is accurate and the fix is minimal and correct. The decision to replace PackageReference ... ExcludeAssets="all" with PackageDownload is well-supported by empirical evidence and by how Dependabot processes NuGet manifests. All supporting machinery (_ValidateBundledSdkFeatureVersions, CopyNuGetPackagesForTestAssets, CPM version literals) continues to work correctly without modification.

One non-blocking inline comment is filed: the parenthetical "also used to stage nupkgs for test assets" in the .csproj comment applies to the Aspire entry only; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Worth tightening in a follow-up if desired, but not a blocker.

- Note that the bundled packages ARE referenced inside the SDK feature targets
(Sdk/Features/*.targets); what's missing is a graph-visible reference from a
restored repo project, which is what Dependabot relies on.
- Clarify that only the Aspire PackageDownload stages a nupkg for test assets;
the Playwright PackageDownload is the Dependabot anchor only.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

🧪 Test quality grade — PR #9452

No new or modified test methods were identified in the changed regions
of this PR. Nothing to grade.

Re-run with /grade-tests.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 90.5 AIC · ⌖ 12.9 AIC · ⊞ 43.8K · [◷]( · )

@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jun 26, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update bundled Playwright for .NET version in MSTest SDK

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix Dependabot discovery of bundled Playwright via PackageDownload - #9452

Merged
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp
Jun 26, 2026
Merged

Fix Dependabot discovery of bundled Playwright via PackageDownload#9452
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Problem

Microsoft.Playwright.MSTest.v4 is still stuck at 1.60.0 even though 1.61.0 has been on the feeds since 2026-06-24 and the anchor fix#9422 has merged. Dependabot keeps updating Aspire.Hosting.Testing but never bumps Playwright (#9362).

Root cause: the #9422 anchor mechanism doesn't work

#9422 added inert anchors as PackageReference ... ExcludeAssets="all". But ExcludeAssets="all" removes the reference from the dependency graph that Dependabot reads, so the package stays invisible to Dependabot and is never proposed for an update — the opposite of the anchor's intent.

The in-repo evidence is a clean A/B from today's Dependabot run (06:47 UTC — after #9422 landed, after 1.61.0 shipped, with 0 open Dependabot PRs so the limit of 15 is irrelevant):

On mainReferenceResult
Aspire.Hosting.TestingPackageDownload+ExcludeAssets="all" anchor✅ bumped 13.2.1 → 13.4.6
Microsoft.Playwright.MSTest.v4ExcludeAssets="all" anchor only❌ stuck at 1.60.0

Same run, same config, same feeds. The only difference is the PackageDownload: Aspire updates via that, not via the ExcludeAssets anchor. Playwright, having only the anchor, is invisible.

Fix

  • Replace the inert ExcludeAssets="all"PackageReference anchors with PackageDownload items for both packages. PackageDownload keeps the package in the restore graph (so Dependabot proposes bumps) without flowing any of its assets (compile/runtime/build — e.g. Playwright's browser install) into the test project. This is the mechanism already proven by Aspire.
  • Correct the now-misleading "condition 2" guidance in Directory.Packages.props and the anchor comment in the acceptance csproj to document that the anchor must be a PackageDownload, not a PackageReference with ExcludeAssets="all".

The _ValidateBundledSdkFeatureVersions drift guard is unchanged and still keeps MicrosoftPlaywrightVersion / AspireHostingTestingVersion in sync with the literal PackageVersion entries.

Validation

MSTest.Acceptance.IntegrationTests restores and builds cleanly (0 warnings, 0 errors).

Notes

This effectively re-applies the PackageDownload approach from the earlier #9449 (which I had closed as "superseded" by #9422 — but #9422's anchor turned out not to work). Supersedes the Playwright-relevant part of #9422.

Closes#9362.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

The orphan `PackageVersion` for `Microsoft.Playwright.MSTest.v4` in
Directory.Packages.props was never enough for Dependabot to discover and
bump it: Dependabot only proposes updates for packages that have a real
consuming reference (PackageReference / PackageDownload). Aspire.Hosting.Testing
sits in the same orphan-CPM ItemGroup but was kept up to date only because it
already had a PackageDownload in the acceptance-test project (see #9446, which
bumped Aspire but left Playwright stuck at 1.60.0).
Add a matching PackageDownload for Microsoft.Playwright.MSTest.v4 and update the
surrounding comments so future bundled SDK feature packages follow the same
pattern.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
main independently fixed#9362 via inert PackageReference ExcludeAssets=\"all\"
anchors for both Playwright and Aspire. Revert this branch's PackageDownload-based
approach and the contradictory comments to match main exactly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The anchor added in #9422 used a PackageReference with ExcludeAssets=\"all\",
which removes the reference from the dependency graph Dependabot reads, so the
package stays invisible and is never bumped. Aspire.Hosting.Testing kept updating
only because it ALSO has a PackageDownload; Microsoft.Playwright.MSTest.v4 had only
the ExcludeAssets anchor and stayed stuck at 1.60.0 (today's Dependabot run bumped
Aspire to 13.4.6 but skipped Playwright, with no PR-limit pressure).
Replace the inert ExcludeAssets anchors with PackageDownload (proven by Aspire),
which keeps the package in the restore graph without flowing any of its assets into
the test project, and correct the now-misleading guidance comments.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 26, 2026 10:57

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the repo’s “Dependabot anchor” mechanism for SDK-bundled feature packages (Playwright + Aspire) so Dependabot can consistently discover and propose version bumps without flowing package assets into the acceptance test project.

Changes:

  • Replaced ExcludeAssets="all"PackageReference anchors with PackageDownload anchors for Aspire.Hosting.Testing and Microsoft.Playwright.MSTest.v4.
  • Consolidated and expanded documentation in the acceptance csproj explaining why PackageDownload is required for Dependabot discovery.
  • Updated Directory.Packages.props guidance to reflect the PackageDownload-based anchoring approach.
Show a summary per file
FileDescription
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csprojSwaps Dependabot anchors to PackageDownload for both bundled feature packages and updates the explanatory comment.
Directory.Packages.propsUpdates documentation explaining Dependabot prerequisites and clarifies that anchors must be PackageDownload.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment threadDirectory.Packages.props Outdated
@Evangelink

This comment has been minimized.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review: PR #9452 — Fix Dependabot discovery of bundled Playwright via PackageDownload

#DimensionSeverityResult
1Algorithmic CorrectnessMAJOR✅ N/A — no algorithmic changes
2Threading & ConcurrencyBLOCKING✅ N/A — no C# code
3SecurityBLOCKING✅ No concerns — only NuGet infrastructure touching well-known packages
4Error HandlingMAJOR_ValidateBundledSdkFeatureVersions guard intact and still fires after a Dependabot bump
5Nullability / Null SafetyMAJOR✅ N/A
6Resource ManagementMAJOR✅ N/A
7API Design & ContractsMAJOR✅ N/A — no public API surface touched
8Test CoverageMINORi️ No automated test can exercise Dependabot's bump behavior; the fix relies on empirical evidence (Aspire kept updating because it had a PackageDownload; Playwright did not because it only had ExcludeAssets="all") — acceptable given the nature of the change
9PerformanceMINORPackageDownload downloads the nupkg to the global cache during restore — marginally more work than the old PackageReference ExcludeAssets="all", but negligible and intentional
10Logging & ObservabilityMINOR✅ N/A
11Configuration & CompatibilityMAJORPackageDownload always requires an explicit version even under CPM — [$(MicrosoftPlaywrightVersion)] and [$(AspireHostingTestingVersion)] are correct: MSBuild evaluates the property first, so NuGet sees [1.60.0] / [13.4.6], both valid exact-version bracket constraints
12Code Clarity & MaintainabilityMINOR✅ Consolidating two separate ItemGroup blocks into one with a unified, well-explained comment is cleaner than the previous split
13Documentation AccuracyMINORi️ See inline comment on line 46 of the .csproj: "also used to stage nupkgs for test assets" is accurate only for the Aspire entry; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Also, both files use the phrase "restore graph" for PackageDownload items — technically these appear under packageDownloads in project.assets.json, not in the dependency/restore graph proper; functionally accurate for Dependabot purposes
14Dependency ManagementMAJOR✅ Core fix is correct. Empirical evidence: Aspire was getting Dependabot bumps (it had a PackageDownload); Playwright was not (it only had PackageReference ExcludeAssets="all"). Switching Playwright to PackageDownload aligns both packages on the working pattern. The Aspire duplicate PackageReference ExcludeAssets="all" is correctly removed as it was redundant
15MSBuild / NuGet CorrectnessMAJORPackageDownload item type, bracket-notation exact-version requirement, and MSBuild property expansion all used correctly. test/Directory.Build.targets adds GeneratePathProperty="True" for Microsoft.Testing.Extensions.CodeCoverage, so $(PkgMicrosoft_Testing_Extensions_CodeCoverage) in CopyNuGetPackagesForTestAssets is unaffected by this change
16Public API SurfaceBLOCKING✅ N/A — build infrastructure only
17CI / CD PipelineMAJOR✅ Dependabot config (directory: "/") scans the entire repo; MSTest.Acceptance.IntegrationTests.csproj is picked up. After this change both Aspire and Playwright entries should receive daily bump proposals
18Layering & ArchitectureMINOR✅ Correct layer to host Dependabot anchors — the acceptance test project participates in the full NuGet restore and is already the canonical home for such anchors (see #9362)
19Dead CodeMINOR✅ The ineffective PackageReference ExcludeAssets="all" anchors are cleanly removed for both packages — no dead items remain
20Breaking ChangesBLOCKING✅ N/A — internal build plumbing; no shipped API surface changes
21Cross-PlatformMINORPackageDownload is NuGet-native and platform-agnostic
22Code Style & FormattingMINOR✅ Alphabetical ordering (Aspire before Playwright) is consistent with existing conventions; comment indentation and line-wrapping match surrounding style

Overall Assessment

The root-cause diagnosis is accurate and the fix is minimal and correct. The decision to replace PackageReference ... ExcludeAssets="all" with PackageDownload is well-supported by empirical evidence and by how Dependabot processes NuGet manifests. All supporting machinery (_ValidateBundledSdkFeatureVersions, CopyNuGetPackagesForTestAssets, CPM version literals) continues to work correctly without modification.

One non-blocking inline comment is filed: the parenthetical "also used to stage nupkgs for test assets" in the .csproj comment applies to the Aspire entry only; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Worth tightening in a follow-up if desired, but not a blocker.

- Note that the bundled packages ARE referenced inside the SDK feature targets
(Sdk/Features/*.targets); what's missing is a graph-visible reference from a
restored repo project, which is what Dependabot relies on.
- Clarify that only the Aspire PackageDownload stages a nupkg for test assets;
the Playwright PackageDownload is the Dependabot anchor only.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

🧪 Test quality grade — PR #9452

No new or modified test methods were identified in the changed regions
of this PR. Nothing to grade.

Re-run with /grade-tests.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 90.5 AIC · ⌖ 12.9 AIC · ⊞ 43.8K · [◷]( · )

@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jun 26, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update bundled Playwright for .NET version in MSTest SDK

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Fix Dependabot discovery of bundled Playwright via PackageDownload - #9452

Merged
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp
Jun 26, 2026
Merged

Fix Dependabot discovery of bundled Playwright via PackageDownload#9452
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Problem

Microsoft.Playwright.MSTest.v4 is still stuck at 1.60.0 even though 1.61.0 has been on the feeds since 2026-06-24 and the anchor fix#9422 has merged. Dependabot keeps updating Aspire.Hosting.Testing but never bumps Playwright (#9362).

Root cause: the #9422 anchor mechanism doesn't work

#9422 added inert anchors as PackageReference ... ExcludeAssets="all". But ExcludeAssets="all" removes the reference from the dependency graph that Dependabot reads, so the package stays invisible to Dependabot and is never proposed for an update — the opposite of the anchor's intent.

The in-repo evidence is a clean A/B from today's Dependabot run (06:47 UTC — after #9422 landed, after 1.61.0 shipped, with 0 open Dependabot PRs so the limit of 15 is irrelevant):

On mainReferenceResult
Aspire.Hosting.TestingPackageDownload+ExcludeAssets="all" anchor✅ bumped 13.2.1 → 13.4.6
Microsoft.Playwright.MSTest.v4ExcludeAssets="all" anchor only❌ stuck at 1.60.0

Same run, same config, same feeds. The only difference is the PackageDownload: Aspire updates via that, not via the ExcludeAssets anchor. Playwright, having only the anchor, is invisible.

Fix

  • Replace the inert ExcludeAssets="all"PackageReference anchors with PackageDownload items for both packages. PackageDownload keeps the package in the restore graph (so Dependabot proposes bumps) without flowing any of its assets (compile/runtime/build — e.g. Playwright's browser install) into the test project. This is the mechanism already proven by Aspire.
  • Correct the now-misleading "condition 2" guidance in Directory.Packages.props and the anchor comment in the acceptance csproj to document that the anchor must be a PackageDownload, not a PackageReference with ExcludeAssets="all".

The _ValidateBundledSdkFeatureVersions drift guard is unchanged and still keeps MicrosoftPlaywrightVersion / AspireHostingTestingVersion in sync with the literal PackageVersion entries.

Validation

MSTest.Acceptance.IntegrationTests restores and builds cleanly (0 warnings, 0 errors).

Notes

This effectively re-applies the PackageDownload approach from the earlier #9449 (which I had closed as "superseded" by #9422 — but #9422's anchor turned out not to work). Supersedes the Playwright-relevant part of #9422.

Closes#9362.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

The orphan `PackageVersion` for `Microsoft.Playwright.MSTest.v4` in
Directory.Packages.props was never enough for Dependabot to discover and
bump it: Dependabot only proposes updates for packages that have a real
consuming reference (PackageReference / PackageDownload). Aspire.Hosting.Testing
sits in the same orphan-CPM ItemGroup but was kept up to date only because it
already had a PackageDownload in the acceptance-test project (see #9446, which
bumped Aspire but left Playwright stuck at 1.60.0).
Add a matching PackageDownload for Microsoft.Playwright.MSTest.v4 and update the
surrounding comments so future bundled SDK feature packages follow the same
pattern.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
main independently fixed#9362 via inert PackageReference ExcludeAssets=\"all\"
anchors for both Playwright and Aspire. Revert this branch's PackageDownload-based
approach and the contradictory comments to match main exactly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The anchor added in #9422 used a PackageReference with ExcludeAssets=\"all\",
which removes the reference from the dependency graph Dependabot reads, so the
package stays invisible and is never bumped. Aspire.Hosting.Testing kept updating
only because it ALSO has a PackageDownload; Microsoft.Playwright.MSTest.v4 had only
the ExcludeAssets anchor and stayed stuck at 1.60.0 (today's Dependabot run bumped
Aspire to 13.4.6 but skipped Playwright, with no PR-limit pressure).
Replace the inert ExcludeAssets anchors with PackageDownload (proven by Aspire),
which keeps the package in the restore graph without flowing any of its assets into
the test project, and correct the now-misleading guidance comments.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 26, 2026 10:57

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the repo’s “Dependabot anchor” mechanism for SDK-bundled feature packages (Playwright + Aspire) so Dependabot can consistently discover and propose version bumps without flowing package assets into the acceptance test project.

Changes:

  • Replaced ExcludeAssets="all"PackageReference anchors with PackageDownload anchors for Aspire.Hosting.Testing and Microsoft.Playwright.MSTest.v4.
  • Consolidated and expanded documentation in the acceptance csproj explaining why PackageDownload is required for Dependabot discovery.
  • Updated Directory.Packages.props guidance to reflect the PackageDownload-based anchoring approach.
Show a summary per file
FileDescription
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csprojSwaps Dependabot anchors to PackageDownload for both bundled feature packages and updates the explanatory comment.
Directory.Packages.propsUpdates documentation explaining Dependabot prerequisites and clarifies that anchors must be PackageDownload.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment threadDirectory.Packages.props Outdated
@Evangelink

This comment has been minimized.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review: PR #9452 — Fix Dependabot discovery of bundled Playwright via PackageDownload

#DimensionSeverityResult
1Algorithmic CorrectnessMAJOR✅ N/A — no algorithmic changes
2Threading & ConcurrencyBLOCKING✅ N/A — no C# code
3SecurityBLOCKING✅ No concerns — only NuGet infrastructure touching well-known packages
4Error HandlingMAJOR_ValidateBundledSdkFeatureVersions guard intact and still fires after a Dependabot bump
5Nullability / Null SafetyMAJOR✅ N/A
6Resource ManagementMAJOR✅ N/A
7API Design & ContractsMAJOR✅ N/A — no public API surface touched
8Test CoverageMINORi️ No automated test can exercise Dependabot's bump behavior; the fix relies on empirical evidence (Aspire kept updating because it had a PackageDownload; Playwright did not because it only had ExcludeAssets="all") — acceptable given the nature of the change
9PerformanceMINORPackageDownload downloads the nupkg to the global cache during restore — marginally more work than the old PackageReference ExcludeAssets="all", but negligible and intentional
10Logging & ObservabilityMINOR✅ N/A
11Configuration & CompatibilityMAJORPackageDownload always requires an explicit version even under CPM — [$(MicrosoftPlaywrightVersion)] and [$(AspireHostingTestingVersion)] are correct: MSBuild evaluates the property first, so NuGet sees [1.60.0] / [13.4.6], both valid exact-version bracket constraints
12Code Clarity & MaintainabilityMINOR✅ Consolidating two separate ItemGroup blocks into one with a unified, well-explained comment is cleaner than the previous split
13Documentation AccuracyMINORi️ See inline comment on line 46 of the .csproj: "also used to stage nupkgs for test assets" is accurate only for the Aspire entry; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Also, both files use the phrase "restore graph" for PackageDownload items — technically these appear under packageDownloads in project.assets.json, not in the dependency/restore graph proper; functionally accurate for Dependabot purposes
14Dependency ManagementMAJOR✅ Core fix is correct. Empirical evidence: Aspire was getting Dependabot bumps (it had a PackageDownload); Playwright was not (it only had PackageReference ExcludeAssets="all"). Switching Playwright to PackageDownload aligns both packages on the working pattern. The Aspire duplicate PackageReference ExcludeAssets="all" is correctly removed as it was redundant
15MSBuild / NuGet CorrectnessMAJORPackageDownload item type, bracket-notation exact-version requirement, and MSBuild property expansion all used correctly. test/Directory.Build.targets adds GeneratePathProperty="True" for Microsoft.Testing.Extensions.CodeCoverage, so $(PkgMicrosoft_Testing_Extensions_CodeCoverage) in CopyNuGetPackagesForTestAssets is unaffected by this change
16Public API SurfaceBLOCKING✅ N/A — build infrastructure only
17CI / CD PipelineMAJOR✅ Dependabot config (directory: "/") scans the entire repo; MSTest.Acceptance.IntegrationTests.csproj is picked up. After this change both Aspire and Playwright entries should receive daily bump proposals
18Layering & ArchitectureMINOR✅ Correct layer to host Dependabot anchors — the acceptance test project participates in the full NuGet restore and is already the canonical home for such anchors (see #9362)
19Dead CodeMINOR✅ The ineffective PackageReference ExcludeAssets="all" anchors are cleanly removed for both packages — no dead items remain
20Breaking ChangesBLOCKING✅ N/A — internal build plumbing; no shipped API surface changes
21Cross-PlatformMINORPackageDownload is NuGet-native and platform-agnostic
22Code Style & FormattingMINOR✅ Alphabetical ordering (Aspire before Playwright) is consistent with existing conventions; comment indentation and line-wrapping match surrounding style

Overall Assessment

The root-cause diagnosis is accurate and the fix is minimal and correct. The decision to replace PackageReference ... ExcludeAssets="all" with PackageDownload is well-supported by empirical evidence and by how Dependabot processes NuGet manifests. All supporting machinery (_ValidateBundledSdkFeatureVersions, CopyNuGetPackagesForTestAssets, CPM version literals) continues to work correctly without modification.

One non-blocking inline comment is filed: the parenthetical "also used to stage nupkgs for test assets" in the .csproj comment applies to the Aspire entry only; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Worth tightening in a follow-up if desired, but not a blocker.

- Note that the bundled packages ARE referenced inside the SDK feature targets
(Sdk/Features/*.targets); what's missing is a graph-visible reference from a
restored repo project, which is what Dependabot relies on.
- Clarify that only the Aspire PackageDownload stages a nupkg for test assets;
the Playwright PackageDownload is the Dependabot anchor only.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

🧪 Test quality grade — PR #9452

No new or modified test methods were identified in the changed regions
of this PR. Nothing to grade.

Re-run with /grade-tests.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 90.5 AIC · ⌖ 12.9 AIC · ⊞ 43.8K · [◷]( · )

@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jun 26, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update bundled Playwright for .NET version in MSTest SDK

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix Dependabot discovery of bundled Playwright via PackageDownload - #9452

Merged
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp
Jun 26, 2026
Merged

Fix Dependabot discovery of bundled Playwright via PackageDownload#9452
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Problem

Microsoft.Playwright.MSTest.v4 is still stuck at 1.60.0 even though 1.61.0 has been on the feeds since 2026-06-24 and the anchor fix#9422 has merged. Dependabot keeps updating Aspire.Hosting.Testing but never bumps Playwright (#9362).

Root cause: the #9422 anchor mechanism doesn't work

#9422 added inert anchors as PackageReference ... ExcludeAssets="all". But ExcludeAssets="all" removes the reference from the dependency graph that Dependabot reads, so the package stays invisible to Dependabot and is never proposed for an update — the opposite of the anchor's intent.

The in-repo evidence is a clean A/B from today's Dependabot run (06:47 UTC — after #9422 landed, after 1.61.0 shipped, with 0 open Dependabot PRs so the limit of 15 is irrelevant):

On mainReferenceResult
Aspire.Hosting.TestingPackageDownload+ExcludeAssets="all" anchor✅ bumped 13.2.1 → 13.4.6
Microsoft.Playwright.MSTest.v4ExcludeAssets="all" anchor only❌ stuck at 1.60.0

Same run, same config, same feeds. The only difference is the PackageDownload: Aspire updates via that, not via the ExcludeAssets anchor. Playwright, having only the anchor, is invisible.

Fix

  • Replace the inert ExcludeAssets="all"PackageReference anchors with PackageDownload items for both packages. PackageDownload keeps the package in the restore graph (so Dependabot proposes bumps) without flowing any of its assets (compile/runtime/build — e.g. Playwright's browser install) into the test project. This is the mechanism already proven by Aspire.
  • Correct the now-misleading "condition 2" guidance in Directory.Packages.props and the anchor comment in the acceptance csproj to document that the anchor must be a PackageDownload, not a PackageReference with ExcludeAssets="all".

The _ValidateBundledSdkFeatureVersions drift guard is unchanged and still keeps MicrosoftPlaywrightVersion / AspireHostingTestingVersion in sync with the literal PackageVersion entries.

Validation

MSTest.Acceptance.IntegrationTests restores and builds cleanly (0 warnings, 0 errors).

Notes

This effectively re-applies the PackageDownload approach from the earlier #9449 (which I had closed as "superseded" by #9422 — but #9422's anchor turned out not to work). Supersedes the Playwright-relevant part of #9422.

Closes#9362.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

The orphan `PackageVersion` for `Microsoft.Playwright.MSTest.v4` in
Directory.Packages.props was never enough for Dependabot to discover and
bump it: Dependabot only proposes updates for packages that have a real
consuming reference (PackageReference / PackageDownload). Aspire.Hosting.Testing
sits in the same orphan-CPM ItemGroup but was kept up to date only because it
already had a PackageDownload in the acceptance-test project (see #9446, which
bumped Aspire but left Playwright stuck at 1.60.0).
Add a matching PackageDownload for Microsoft.Playwright.MSTest.v4 and update the
surrounding comments so future bundled SDK feature packages follow the same
pattern.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
main independently fixed#9362 via inert PackageReference ExcludeAssets=\"all\"
anchors for both Playwright and Aspire. Revert this branch's PackageDownload-based
approach and the contradictory comments to match main exactly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The anchor added in #9422 used a PackageReference with ExcludeAssets=\"all\",
which removes the reference from the dependency graph Dependabot reads, so the
package stays invisible and is never bumped. Aspire.Hosting.Testing kept updating
only because it ALSO has a PackageDownload; Microsoft.Playwright.MSTest.v4 had only
the ExcludeAssets anchor and stayed stuck at 1.60.0 (today's Dependabot run bumped
Aspire to 13.4.6 but skipped Playwright, with no PR-limit pressure).
Replace the inert ExcludeAssets anchors with PackageDownload (proven by Aspire),
which keeps the package in the restore graph without flowing any of its assets into
the test project, and correct the now-misleading guidance comments.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 26, 2026 10:57

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the repo’s “Dependabot anchor” mechanism for SDK-bundled feature packages (Playwright + Aspire) so Dependabot can consistently discover and propose version bumps without flowing package assets into the acceptance test project.

Changes:

  • Replaced ExcludeAssets="all"PackageReference anchors with PackageDownload anchors for Aspire.Hosting.Testing and Microsoft.Playwright.MSTest.v4.
  • Consolidated and expanded documentation in the acceptance csproj explaining why PackageDownload is required for Dependabot discovery.
  • Updated Directory.Packages.props guidance to reflect the PackageDownload-based anchoring approach.
Show a summary per file
FileDescription
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csprojSwaps Dependabot anchors to PackageDownload for both bundled feature packages and updates the explanatory comment.
Directory.Packages.propsUpdates documentation explaining Dependabot prerequisites and clarifies that anchors must be PackageDownload.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment threadDirectory.Packages.props Outdated
@Evangelink

This comment has been minimized.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review: PR #9452 — Fix Dependabot discovery of bundled Playwright via PackageDownload

#DimensionSeverityResult
1Algorithmic CorrectnessMAJOR✅ N/A — no algorithmic changes
2Threading & ConcurrencyBLOCKING✅ N/A — no C# code
3SecurityBLOCKING✅ No concerns — only NuGet infrastructure touching well-known packages
4Error HandlingMAJOR_ValidateBundledSdkFeatureVersions guard intact and still fires after a Dependabot bump
5Nullability / Null SafetyMAJOR✅ N/A
6Resource ManagementMAJOR✅ N/A
7API Design & ContractsMAJOR✅ N/A — no public API surface touched
8Test CoverageMINORi️ No automated test can exercise Dependabot's bump behavior; the fix relies on empirical evidence (Aspire kept updating because it had a PackageDownload; Playwright did not because it only had ExcludeAssets="all") — acceptable given the nature of the change
9PerformanceMINORPackageDownload downloads the nupkg to the global cache during restore — marginally more work than the old PackageReference ExcludeAssets="all", but negligible and intentional
10Logging & ObservabilityMINOR✅ N/A
11Configuration & CompatibilityMAJORPackageDownload always requires an explicit version even under CPM — [$(MicrosoftPlaywrightVersion)] and [$(AspireHostingTestingVersion)] are correct: MSBuild evaluates the property first, so NuGet sees [1.60.0] / [13.4.6], both valid exact-version bracket constraints
12Code Clarity & MaintainabilityMINOR✅ Consolidating two separate ItemGroup blocks into one with a unified, well-explained comment is cleaner than the previous split
13Documentation AccuracyMINORi️ See inline comment on line 46 of the .csproj: "also used to stage nupkgs for test assets" is accurate only for the Aspire entry; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Also, both files use the phrase "restore graph" for PackageDownload items — technically these appear under packageDownloads in project.assets.json, not in the dependency/restore graph proper; functionally accurate for Dependabot purposes
14Dependency ManagementMAJOR✅ Core fix is correct. Empirical evidence: Aspire was getting Dependabot bumps (it had a PackageDownload); Playwright was not (it only had PackageReference ExcludeAssets="all"). Switching Playwright to PackageDownload aligns both packages on the working pattern. The Aspire duplicate PackageReference ExcludeAssets="all" is correctly removed as it was redundant
15MSBuild / NuGet CorrectnessMAJORPackageDownload item type, bracket-notation exact-version requirement, and MSBuild property expansion all used correctly. test/Directory.Build.targets adds GeneratePathProperty="True" for Microsoft.Testing.Extensions.CodeCoverage, so $(PkgMicrosoft_Testing_Extensions_CodeCoverage) in CopyNuGetPackagesForTestAssets is unaffected by this change
16Public API SurfaceBLOCKING✅ N/A — build infrastructure only
17CI / CD PipelineMAJOR✅ Dependabot config (directory: "/") scans the entire repo; MSTest.Acceptance.IntegrationTests.csproj is picked up. After this change both Aspire and Playwright entries should receive daily bump proposals
18Layering & ArchitectureMINOR✅ Correct layer to host Dependabot anchors — the acceptance test project participates in the full NuGet restore and is already the canonical home for such anchors (see #9362)
19Dead CodeMINOR✅ The ineffective PackageReference ExcludeAssets="all" anchors are cleanly removed for both packages — no dead items remain
20Breaking ChangesBLOCKING✅ N/A — internal build plumbing; no shipped API surface changes
21Cross-PlatformMINORPackageDownload is NuGet-native and platform-agnostic
22Code Style & FormattingMINOR✅ Alphabetical ordering (Aspire before Playwright) is consistent with existing conventions; comment indentation and line-wrapping match surrounding style

Overall Assessment

The root-cause diagnosis is accurate and the fix is minimal and correct. The decision to replace PackageReference ... ExcludeAssets="all" with PackageDownload is well-supported by empirical evidence and by how Dependabot processes NuGet manifests. All supporting machinery (_ValidateBundledSdkFeatureVersions, CopyNuGetPackagesForTestAssets, CPM version literals) continues to work correctly without modification.

One non-blocking inline comment is filed: the parenthetical "also used to stage nupkgs for test assets" in the .csproj comment applies to the Aspire entry only; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Worth tightening in a follow-up if desired, but not a blocker.

- Note that the bundled packages ARE referenced inside the SDK feature targets
(Sdk/Features/*.targets); what's missing is a graph-visible reference from a
restored repo project, which is what Dependabot relies on.
- Clarify that only the Aspire PackageDownload stages a nupkg for test assets;
the Playwright PackageDownload is the Dependabot anchor only.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

🧪 Test quality grade — PR #9452

No new or modified test methods were identified in the changed regions
of this PR. Nothing to grade.

Re-run with /grade-tests.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 90.5 AIC · ⌖ 12.9 AIC · ⊞ 43.8K · [◷]( · )

@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jun 26, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update bundled Playwright for .NET version in MSTest SDK

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix Dependabot discovery of bundled Playwright via PackageDownload - #9452

Merged
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp
Jun 26, 2026
Merged

Fix Dependabot discovery of bundled Playwright via PackageDownload#9452
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Problem

Microsoft.Playwright.MSTest.v4 is still stuck at 1.60.0 even though 1.61.0 has been on the feeds since 2026-06-24 and the anchor fix#9422 has merged. Dependabot keeps updating Aspire.Hosting.Testing but never bumps Playwright (#9362).

Root cause: the #9422 anchor mechanism doesn't work

#9422 added inert anchors as PackageReference ... ExcludeAssets="all". But ExcludeAssets="all" removes the reference from the dependency graph that Dependabot reads, so the package stays invisible to Dependabot and is never proposed for an update — the opposite of the anchor's intent.

The in-repo evidence is a clean A/B from today's Dependabot run (06:47 UTC — after #9422 landed, after 1.61.0 shipped, with 0 open Dependabot PRs so the limit of 15 is irrelevant):

On mainReferenceResult
Aspire.Hosting.TestingPackageDownload+ExcludeAssets="all" anchor✅ bumped 13.2.1 → 13.4.6
Microsoft.Playwright.MSTest.v4ExcludeAssets="all" anchor only❌ stuck at 1.60.0

Same run, same config, same feeds. The only difference is the PackageDownload: Aspire updates via that, not via the ExcludeAssets anchor. Playwright, having only the anchor, is invisible.

Fix

  • Replace the inert ExcludeAssets="all"PackageReference anchors with PackageDownload items for both packages. PackageDownload keeps the package in the restore graph (so Dependabot proposes bumps) without flowing any of its assets (compile/runtime/build — e.g. Playwright's browser install) into the test project. This is the mechanism already proven by Aspire.
  • Correct the now-misleading "condition 2" guidance in Directory.Packages.props and the anchor comment in the acceptance csproj to document that the anchor must be a PackageDownload, not a PackageReference with ExcludeAssets="all".

The _ValidateBundledSdkFeatureVersions drift guard is unchanged and still keeps MicrosoftPlaywrightVersion / AspireHostingTestingVersion in sync with the literal PackageVersion entries.

Validation

MSTest.Acceptance.IntegrationTests restores and builds cleanly (0 warnings, 0 errors).

Notes

This effectively re-applies the PackageDownload approach from the earlier #9449 (which I had closed as "superseded" by #9422 — but #9422's anchor turned out not to work). Supersedes the Playwright-relevant part of #9422.

Closes#9362.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

The orphan `PackageVersion` for `Microsoft.Playwright.MSTest.v4` in
Directory.Packages.props was never enough for Dependabot to discover and
bump it: Dependabot only proposes updates for packages that have a real
consuming reference (PackageReference / PackageDownload). Aspire.Hosting.Testing
sits in the same orphan-CPM ItemGroup but was kept up to date only because it
already had a PackageDownload in the acceptance-test project (see #9446, which
bumped Aspire but left Playwright stuck at 1.60.0).
Add a matching PackageDownload for Microsoft.Playwright.MSTest.v4 and update the
surrounding comments so future bundled SDK feature packages follow the same
pattern.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
main independently fixed#9362 via inert PackageReference ExcludeAssets=\"all\"
anchors for both Playwright and Aspire. Revert this branch's PackageDownload-based
approach and the contradictory comments to match main exactly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The anchor added in #9422 used a PackageReference with ExcludeAssets=\"all\",
which removes the reference from the dependency graph Dependabot reads, so the
package stays invisible and is never bumped. Aspire.Hosting.Testing kept updating
only because it ALSO has a PackageDownload; Microsoft.Playwright.MSTest.v4 had only
the ExcludeAssets anchor and stayed stuck at 1.60.0 (today's Dependabot run bumped
Aspire to 13.4.6 but skipped Playwright, with no PR-limit pressure).
Replace the inert ExcludeAssets anchors with PackageDownload (proven by Aspire),
which keeps the package in the restore graph without flowing any of its assets into
the test project, and correct the now-misleading guidance comments.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 26, 2026 10:57

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the repo’s “Dependabot anchor” mechanism for SDK-bundled feature packages (Playwright + Aspire) so Dependabot can consistently discover and propose version bumps without flowing package assets into the acceptance test project.

Changes:

  • Replaced ExcludeAssets="all"PackageReference anchors with PackageDownload anchors for Aspire.Hosting.Testing and Microsoft.Playwright.MSTest.v4.
  • Consolidated and expanded documentation in the acceptance csproj explaining why PackageDownload is required for Dependabot discovery.
  • Updated Directory.Packages.props guidance to reflect the PackageDownload-based anchoring approach.
Show a summary per file
FileDescription
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csprojSwaps Dependabot anchors to PackageDownload for both bundled feature packages and updates the explanatory comment.
Directory.Packages.propsUpdates documentation explaining Dependabot prerequisites and clarifies that anchors must be PackageDownload.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment threadDirectory.Packages.props Outdated
@Evangelink

This comment has been minimized.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review: PR #9452 — Fix Dependabot discovery of bundled Playwright via PackageDownload

#DimensionSeverityResult
1Algorithmic CorrectnessMAJOR✅ N/A — no algorithmic changes
2Threading & ConcurrencyBLOCKING✅ N/A — no C# code
3SecurityBLOCKING✅ No concerns — only NuGet infrastructure touching well-known packages
4Error HandlingMAJOR_ValidateBundledSdkFeatureVersions guard intact and still fires after a Dependabot bump
5Nullability / Null SafetyMAJOR✅ N/A
6Resource ManagementMAJOR✅ N/A
7API Design & ContractsMAJOR✅ N/A — no public API surface touched
8Test CoverageMINORi️ No automated test can exercise Dependabot's bump behavior; the fix relies on empirical evidence (Aspire kept updating because it had a PackageDownload; Playwright did not because it only had ExcludeAssets="all") — acceptable given the nature of the change
9PerformanceMINORPackageDownload downloads the nupkg to the global cache during restore — marginally more work than the old PackageReference ExcludeAssets="all", but negligible and intentional
10Logging & ObservabilityMINOR✅ N/A
11Configuration & CompatibilityMAJORPackageDownload always requires an explicit version even under CPM — [$(MicrosoftPlaywrightVersion)] and [$(AspireHostingTestingVersion)] are correct: MSBuild evaluates the property first, so NuGet sees [1.60.0] / [13.4.6], both valid exact-version bracket constraints
12Code Clarity & MaintainabilityMINOR✅ Consolidating two separate ItemGroup blocks into one with a unified, well-explained comment is cleaner than the previous split
13Documentation AccuracyMINORi️ See inline comment on line 46 of the .csproj: "also used to stage nupkgs for test assets" is accurate only for the Aspire entry; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Also, both files use the phrase "restore graph" for PackageDownload items — technically these appear under packageDownloads in project.assets.json, not in the dependency/restore graph proper; functionally accurate for Dependabot purposes
14Dependency ManagementMAJOR✅ Core fix is correct. Empirical evidence: Aspire was getting Dependabot bumps (it had a PackageDownload); Playwright was not (it only had PackageReference ExcludeAssets="all"). Switching Playwright to PackageDownload aligns both packages on the working pattern. The Aspire duplicate PackageReference ExcludeAssets="all" is correctly removed as it was redundant
15MSBuild / NuGet CorrectnessMAJORPackageDownload item type, bracket-notation exact-version requirement, and MSBuild property expansion all used correctly. test/Directory.Build.targets adds GeneratePathProperty="True" for Microsoft.Testing.Extensions.CodeCoverage, so $(PkgMicrosoft_Testing_Extensions_CodeCoverage) in CopyNuGetPackagesForTestAssets is unaffected by this change
16Public API SurfaceBLOCKING✅ N/A — build infrastructure only
17CI / CD PipelineMAJOR✅ Dependabot config (directory: "/") scans the entire repo; MSTest.Acceptance.IntegrationTests.csproj is picked up. After this change both Aspire and Playwright entries should receive daily bump proposals
18Layering & ArchitectureMINOR✅ Correct layer to host Dependabot anchors — the acceptance test project participates in the full NuGet restore and is already the canonical home for such anchors (see #9362)
19Dead CodeMINOR✅ The ineffective PackageReference ExcludeAssets="all" anchors are cleanly removed for both packages — no dead items remain
20Breaking ChangesBLOCKING✅ N/A — internal build plumbing; no shipped API surface changes
21Cross-PlatformMINORPackageDownload is NuGet-native and platform-agnostic
22Code Style & FormattingMINOR✅ Alphabetical ordering (Aspire before Playwright) is consistent with existing conventions; comment indentation and line-wrapping match surrounding style

Overall Assessment

The root-cause diagnosis is accurate and the fix is minimal and correct. The decision to replace PackageReference ... ExcludeAssets="all" with PackageDownload is well-supported by empirical evidence and by how Dependabot processes NuGet manifests. All supporting machinery (_ValidateBundledSdkFeatureVersions, CopyNuGetPackagesForTestAssets, CPM version literals) continues to work correctly without modification.

One non-blocking inline comment is filed: the parenthetical "also used to stage nupkgs for test assets" in the .csproj comment applies to the Aspire entry only; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Worth tightening in a follow-up if desired, but not a blocker.

- Note that the bundled packages ARE referenced inside the SDK feature targets
(Sdk/Features/*.targets); what's missing is a graph-visible reference from a
restored repo project, which is what Dependabot relies on.
- Clarify that only the Aspire PackageDownload stages a nupkg for test assets;
the Playwright PackageDownload is the Dependabot anchor only.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

🧪 Test quality grade — PR #9452

No new or modified test methods were identified in the changed regions
of this PR. Nothing to grade.

Re-run with /grade-tests.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 90.5 AIC · ⌖ 12.9 AIC · ⊞ 43.8K · [◷]( · )

@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jun 26, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update bundled Playwright for .NET version in MSTest SDK

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Fix Dependabot discovery of bundled Playwright via PackageDownload - #9452

Merged
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp
Jun 26, 2026
Merged

Fix Dependabot discovery of bundled Playwright via PackageDownload#9452
Amaury Levé (Evangelink) merged 5 commits into
mainfrom
evangelink-supreme-lamp

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Problem

Microsoft.Playwright.MSTest.v4 is still stuck at 1.60.0 even though 1.61.0 has been on the feeds since 2026-06-24 and the anchor fix#9422 has merged. Dependabot keeps updating Aspire.Hosting.Testing but never bumps Playwright (#9362).

Root cause: the #9422 anchor mechanism doesn't work

#9422 added inert anchors as PackageReference ... ExcludeAssets="all". But ExcludeAssets="all" removes the reference from the dependency graph that Dependabot reads, so the package stays invisible to Dependabot and is never proposed for an update — the opposite of the anchor's intent.

The in-repo evidence is a clean A/B from today's Dependabot run (06:47 UTC — after #9422 landed, after 1.61.0 shipped, with 0 open Dependabot PRs so the limit of 15 is irrelevant):

On mainReferenceResult
Aspire.Hosting.TestingPackageDownload+ExcludeAssets="all" anchor✅ bumped 13.2.1 → 13.4.6
Microsoft.Playwright.MSTest.v4ExcludeAssets="all" anchor only❌ stuck at 1.60.0

Same run, same config, same feeds. The only difference is the PackageDownload: Aspire updates via that, not via the ExcludeAssets anchor. Playwright, having only the anchor, is invisible.

Fix

  • Replace the inert ExcludeAssets="all"PackageReference anchors with PackageDownload items for both packages. PackageDownload keeps the package in the restore graph (so Dependabot proposes bumps) without flowing any of its assets (compile/runtime/build — e.g. Playwright's browser install) into the test project. This is the mechanism already proven by Aspire.
  • Correct the now-misleading "condition 2" guidance in Directory.Packages.props and the anchor comment in the acceptance csproj to document that the anchor must be a PackageDownload, not a PackageReference with ExcludeAssets="all".

The _ValidateBundledSdkFeatureVersions drift guard is unchanged and still keeps MicrosoftPlaywrightVersion / AspireHostingTestingVersion in sync with the literal PackageVersion entries.

Validation

MSTest.Acceptance.IntegrationTests restores and builds cleanly (0 warnings, 0 errors).

Notes

This effectively re-applies the PackageDownload approach from the earlier #9449 (which I had closed as "superseded" by #9422 — but #9422's anchor turned out not to work). Supersedes the Playwright-relevant part of #9422.

Closes#9362.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

The orphan `PackageVersion` for `Microsoft.Playwright.MSTest.v4` in
Directory.Packages.props was never enough for Dependabot to discover and
bump it: Dependabot only proposes updates for packages that have a real
consuming reference (PackageReference / PackageDownload). Aspire.Hosting.Testing
sits in the same orphan-CPM ItemGroup but was kept up to date only because it
already had a PackageDownload in the acceptance-test project (see #9446, which
bumped Aspire but left Playwright stuck at 1.60.0).
Add a matching PackageDownload for Microsoft.Playwright.MSTest.v4 and update the
surrounding comments so future bundled SDK feature packages follow the same
pattern.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
main independently fixed#9362 via inert PackageReference ExcludeAssets=\"all\"
anchors for both Playwright and Aspire. Revert this branch's PackageDownload-based
approach and the contradictory comments to match main exactly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The anchor added in #9422 used a PackageReference with ExcludeAssets=\"all\",
which removes the reference from the dependency graph Dependabot reads, so the
package stays invisible and is never bumped. Aspire.Hosting.Testing kept updating
only because it ALSO has a PackageDownload; Microsoft.Playwright.MSTest.v4 had only
the ExcludeAssets anchor and stayed stuck at 1.60.0 (today's Dependabot run bumped
Aspire to 13.4.6 but skipped Playwright, with no PR-limit pressure).
Replace the inert ExcludeAssets anchors with PackageDownload (proven by Aspire),
which keeps the package in the restore graph without flowing any of its assets into
the test project, and correct the now-misleading guidance comments.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 26, 2026 10:57

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the repo’s “Dependabot anchor” mechanism for SDK-bundled feature packages (Playwright + Aspire) so Dependabot can consistently discover and propose version bumps without flowing package assets into the acceptance test project.

Changes:

  • Replaced ExcludeAssets="all"PackageReference anchors with PackageDownload anchors for Aspire.Hosting.Testing and Microsoft.Playwright.MSTest.v4.
  • Consolidated and expanded documentation in the acceptance csproj explaining why PackageDownload is required for Dependabot discovery.
  • Updated Directory.Packages.props guidance to reflect the PackageDownload-based anchoring approach.
Show a summary per file
FileDescription
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/MSTest.Acceptance.IntegrationTests.csprojSwaps Dependabot anchors to PackageDownload for both bundled feature packages and updates the explanatory comment.
Directory.Packages.propsUpdates documentation explaining Dependabot prerequisites and clarifies that anchors must be PackageDownload.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment threadDirectory.Packages.props Outdated
@Evangelink

This comment has been minimized.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Review: PR #9452 — Fix Dependabot discovery of bundled Playwright via PackageDownload

#DimensionSeverityResult
1Algorithmic CorrectnessMAJOR✅ N/A — no algorithmic changes
2Threading & ConcurrencyBLOCKING✅ N/A — no C# code
3SecurityBLOCKING✅ No concerns — only NuGet infrastructure touching well-known packages
4Error HandlingMAJOR_ValidateBundledSdkFeatureVersions guard intact and still fires after a Dependabot bump
5Nullability / Null SafetyMAJOR✅ N/A
6Resource ManagementMAJOR✅ N/A
7API Design & ContractsMAJOR✅ N/A — no public API surface touched
8Test CoverageMINORi️ No automated test can exercise Dependabot's bump behavior; the fix relies on empirical evidence (Aspire kept updating because it had a PackageDownload; Playwright did not because it only had ExcludeAssets="all") — acceptable given the nature of the change
9PerformanceMINORPackageDownload downloads the nupkg to the global cache during restore — marginally more work than the old PackageReference ExcludeAssets="all", but negligible and intentional
10Logging & ObservabilityMINOR✅ N/A
11Configuration & CompatibilityMAJORPackageDownload always requires an explicit version even under CPM — [$(MicrosoftPlaywrightVersion)] and [$(AspireHostingTestingVersion)] are correct: MSBuild evaluates the property first, so NuGet sees [1.60.0] / [13.4.6], both valid exact-version bracket constraints
12Code Clarity & MaintainabilityMINOR✅ Consolidating two separate ItemGroup blocks into one with a unified, well-explained comment is cleaner than the previous split
13Documentation AccuracyMINORi️ See inline comment on line 46 of the .csproj: "also used to stage nupkgs for test assets" is accurate only for the Aspire entry; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Also, both files use the phrase "restore graph" for PackageDownload items — technically these appear under packageDownloads in project.assets.json, not in the dependency/restore graph proper; functionally accurate for Dependabot purposes
14Dependency ManagementMAJOR✅ Core fix is correct. Empirical evidence: Aspire was getting Dependabot bumps (it had a PackageDownload); Playwright was not (it only had PackageReference ExcludeAssets="all"). Switching Playwright to PackageDownload aligns both packages on the working pattern. The Aspire duplicate PackageReference ExcludeAssets="all" is correctly removed as it was redundant
15MSBuild / NuGet CorrectnessMAJORPackageDownload item type, bracket-notation exact-version requirement, and MSBuild property expansion all used correctly. test/Directory.Build.targets adds GeneratePathProperty="True" for Microsoft.Testing.Extensions.CodeCoverage, so $(PkgMicrosoft_Testing_Extensions_CodeCoverage) in CopyNuGetPackagesForTestAssets is unaffected by this change
16Public API SurfaceBLOCKING✅ N/A — build infrastructure only
17CI / CD PipelineMAJOR✅ Dependabot config (directory: "/") scans the entire repo; MSTest.Acceptance.IntegrationTests.csproj is picked up. After this change both Aspire and Playwright entries should receive daily bump proposals
18Layering & ArchitectureMINOR✅ Correct layer to host Dependabot anchors — the acceptance test project participates in the full NuGet restore and is already the canonical home for such anchors (see #9362)
19Dead CodeMINOR✅ The ineffective PackageReference ExcludeAssets="all" anchors are cleanly removed for both packages — no dead items remain
20Breaking ChangesBLOCKING✅ N/A — internal build plumbing; no shipped API surface changes
21Cross-PlatformMINORPackageDownload is NuGet-native and platform-agnostic
22Code Style & FormattingMINOR✅ Alphabetical ordering (Aspire before Playwright) is consistent with existing conventions; comment indentation and line-wrapping match surrounding style

Overall Assessment

The root-cause diagnosis is accurate and the fix is minimal and correct. The decision to replace PackageReference ... ExcludeAssets="all" with PackageDownload is well-supported by empirical evidence and by how Dependabot processes NuGet manifests. All supporting machinery (_ValidateBundledSdkFeatureVersions, CopyNuGetPackagesForTestAssets, CPM version literals) continues to work correctly without modification.

One non-blocking inline comment is filed: the parenthetical "also used to stage nupkgs for test assets" in the .csproj comment applies to the Aspire entry only; the Playwright PackageDownload never feeds CopyNuGetPackagesForTestAssets. Worth tightening in a follow-up if desired, but not a blocker.

- Note that the bundled packages ARE referenced inside the SDK feature targets
(Sdk/Features/*.targets); what's missing is a graph-visible reference from a
restored repo project, which is what Dependabot relies on.
- Clarify that only the Aspire PackageDownload stages a nupkg for test assets;
the Playwright PackageDownload is the Dependabot anchor only.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

🧪 Test quality grade — PR #9452

No new or modified test methods were identified in the changed regions
of this PR. Nothing to grade.

Re-run with /grade-tests.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 90.5 AIC · ⌖ 12.9 AIC · ⊞ 43.8K · [◷]( · )

@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jun 26, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-reviewAwaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update bundled Playwright for .NET version in MSTest SDK

3 participants

@Evangelink@0101