Simplify GitHubActionsAnnotationReporter - #9677

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo
Jul 7, 2026
Merged

Simplify GitHubActionsAnnotationReporter#9677
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9658.

Two small, purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (added in #9641):

  1. Eliminated the duplicate GetTestName call in ConsumeAsync — extracted a testName local computed once before the failure/skip branch.
  2. Extracted a shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, which both ended with an identical _outputDisplay.DisplayAsync(...) call plus the same leading-newline explanation. The helper centralises the call and its full explanation; both write methods now return the helper's Task directly (no longer async).

No functional changes.

Testing

  • Microsoft.Testing.Extensions.GitHubActionsReport builds with 0 warnings / 0 errors across all target frameworks.
  • All Microsoft.Testing.Extensions.UnitTests (including GitHubActionsAnnotationReporterTests) pass unchanged on net8.0, net9.0, net462, and net472.

Extract testName local in ConsumeAsync to avoid the duplicate GetTestName call, and extract the shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, consolidating the leading-newline explanation in one place.
Fixes#9658
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 7, 2026 04:32
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jul 7, 2026

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

This PR applies two purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (originally added in #9641). It removes a duplicated GetTestName call in ConsumeAsync and extracts the identical output-display logic (plus its explanatory comment) shared by the failure and skip write paths into a single DisplayAnnotationLineAsync helper. The extension is a Microsoft.Testing.Platform reporter that emits GitHub Actions ::error/::warning workflow-command annotations, and these changes reduce duplication without altering the emitted annotations.

Changes:

  • Extracted a testName local in ConsumeAsync, computed once instead of per branch.
  • Added a private DisplayAnnotationLineAsync helper that centralizes the _outputDisplay.DisplayAsync(...) call and the leading-newline explanation.
  • Converted WriteAnnotationAsync/WriteSkippedAnnotationAsync from async Task to returning the helper's Task directly (callers retain .ConfigureAwait(false)).
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.csDeduplicates the GetTestName call and factors the shared annotation-display logic into a new DisplayAnnotationLineAsync helper; both write methods now return the helper's task directly.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Medium

@github-actionsgithub-actionsBot 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.

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.

✅ 22/22 dimensions clean — no findings.

Clean mechanical refactoring: the testName hoist is safe (GetTestName is a static delegate with no side effects), and the async removal is correct since all callers await inside a try/catch — synchronous propagation from the non-async path reaches the same handler as a faulted Task would.

@Evangelink
Amaury Levé (Evangelink) enabled auto-merge (squash) July 7, 2026 07:38
…inistic
- Address review feedback: revert the hoisted 'testName' local so GetTestName
stays lazy at the two annotation call sites, avoiding an eager property-bag
walk + string allocation on the common passing/in-progress path (keeps the
DisplayAnnotationLineAsync extraction).
- Fix flaky Windows Release CI: GetErrorAnnotation source-location tests relied
on a real throw whose PDB-derived line shifts under Release JIT (net462 vs
net472), producing line=110 vs expected 114. Build a deterministic synthetic
stack frame via [CallerFilePath] instead, keeping both file and line stable
while still exercising the resolver's repo-root/'/_/' relativization.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

# Conflicts:
#	src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.cs
CopilotAI review requested due to automatic review settings July 7, 2026 08:40

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.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9677

GradeTestNotes
A (90–100)GitHubActionsAnnotationReporterTests.
GetSkippedAnnotation_
FallsBackToDefaultReason_
WhenNoExplanation
Clear AAA, exact equality assertion validates the complete output string including percent-encoding and the precise fallback message; no issues found.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. 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. · 89.2 AIC · ⌖ 7.06 AIC · ⊞ 9.5K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit c6ad50d into mainJul 7, 2026
39 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/silver-garbanzo branch July 7, 2026 11:18
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.

[code-simplifier] simplify: extract testName variable and DisplayAnnotationLineAsync helper in GitHubActionsAnnotationReporter

3 participants

@Evangelink@0101
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 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

Simplify GitHubActionsAnnotationReporter - #9677

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo
Jul 7, 2026
Merged

Simplify GitHubActionsAnnotationReporter#9677
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9658.

Two small, purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (added in #9641):

  1. Eliminated the duplicate GetTestName call in ConsumeAsync — extracted a testName local computed once before the failure/skip branch.
  2. Extracted a shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, which both ended with an identical _outputDisplay.DisplayAsync(...) call plus the same leading-newline explanation. The helper centralises the call and its full explanation; both write methods now return the helper's Task directly (no longer async).

No functional changes.

Testing

  • Microsoft.Testing.Extensions.GitHubActionsReport builds with 0 warnings / 0 errors across all target frameworks.
  • All Microsoft.Testing.Extensions.UnitTests (including GitHubActionsAnnotationReporterTests) pass unchanged on net8.0, net9.0, net462, and net472.

Extract testName local in ConsumeAsync to avoid the duplicate GetTestName call, and extract the shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, consolidating the leading-newline explanation in one place.
Fixes#9658
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 7, 2026 04:32
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jul 7, 2026

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

This PR applies two purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (originally added in #9641). It removes a duplicated GetTestName call in ConsumeAsync and extracts the identical output-display logic (plus its explanatory comment) shared by the failure and skip write paths into a single DisplayAnnotationLineAsync helper. The extension is a Microsoft.Testing.Platform reporter that emits GitHub Actions ::error/::warning workflow-command annotations, and these changes reduce duplication without altering the emitted annotations.

Changes:

  • Extracted a testName local in ConsumeAsync, computed once instead of per branch.
  • Added a private DisplayAnnotationLineAsync helper that centralizes the _outputDisplay.DisplayAsync(...) call and the leading-newline explanation.
  • Converted WriteAnnotationAsync/WriteSkippedAnnotationAsync from async Task to returning the helper's Task directly (callers retain .ConfigureAwait(false)).
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.csDeduplicates the GetTestName call and factors the shared annotation-display logic into a new DisplayAnnotationLineAsync helper; both write methods now return the helper's task directly.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Medium

@github-actionsgithub-actionsBot 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.

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.

✅ 22/22 dimensions clean — no findings.

Clean mechanical refactoring: the testName hoist is safe (GetTestName is a static delegate with no side effects), and the async removal is correct since all callers await inside a try/catch — synchronous propagation from the non-async path reaches the same handler as a faulted Task would.

@Evangelink
Amaury Levé (Evangelink) enabled auto-merge (squash) July 7, 2026 07:38
…inistic
- Address review feedback: revert the hoisted 'testName' local so GetTestName
stays lazy at the two annotation call sites, avoiding an eager property-bag
walk + string allocation on the common passing/in-progress path (keeps the
DisplayAnnotationLineAsync extraction).
- Fix flaky Windows Release CI: GetErrorAnnotation source-location tests relied
on a real throw whose PDB-derived line shifts under Release JIT (net462 vs
net472), producing line=110 vs expected 114. Build a deterministic synthetic
stack frame via [CallerFilePath] instead, keeping both file and line stable
while still exercising the resolver's repo-root/'/_/' relativization.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

# Conflicts:
#	src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.cs
CopilotAI review requested due to automatic review settings July 7, 2026 08:40

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.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9677

GradeTestNotes
A (90–100)GitHubActionsAnnotationReporterTests.
GetSkippedAnnotation_
FallsBackToDefaultReason_
WhenNoExplanation
Clear AAA, exact equality assertion validates the complete output string including percent-encoding and the precise fallback message; no issues found.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. 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. · 89.2 AIC · ⌖ 7.06 AIC · ⊞ 9.5K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit c6ad50d into mainJul 7, 2026
39 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/silver-garbanzo branch July 7, 2026 11:18
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.

[code-simplifier] simplify: extract testName variable and DisplayAnnotationLineAsync helper in GitHubActionsAnnotationReporter

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

Simplify GitHubActionsAnnotationReporter - #9677

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo
Jul 7, 2026
Merged

Simplify GitHubActionsAnnotationReporter#9677
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9658.

Two small, purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (added in #9641):

  1. Eliminated the duplicate GetTestName call in ConsumeAsync — extracted a testName local computed once before the failure/skip branch.
  2. Extracted a shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, which both ended with an identical _outputDisplay.DisplayAsync(...) call plus the same leading-newline explanation. The helper centralises the call and its full explanation; both write methods now return the helper's Task directly (no longer async).

No functional changes.

Testing

  • Microsoft.Testing.Extensions.GitHubActionsReport builds with 0 warnings / 0 errors across all target frameworks.
  • All Microsoft.Testing.Extensions.UnitTests (including GitHubActionsAnnotationReporterTests) pass unchanged on net8.0, net9.0, net462, and net472.

Extract testName local in ConsumeAsync to avoid the duplicate GetTestName call, and extract the shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, consolidating the leading-newline explanation in one place.
Fixes#9658
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 7, 2026 04:32
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jul 7, 2026

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

This PR applies two purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (originally added in #9641). It removes a duplicated GetTestName call in ConsumeAsync and extracts the identical output-display logic (plus its explanatory comment) shared by the failure and skip write paths into a single DisplayAnnotationLineAsync helper. The extension is a Microsoft.Testing.Platform reporter that emits GitHub Actions ::error/::warning workflow-command annotations, and these changes reduce duplication without altering the emitted annotations.

Changes:

  • Extracted a testName local in ConsumeAsync, computed once instead of per branch.
  • Added a private DisplayAnnotationLineAsync helper that centralizes the _outputDisplay.DisplayAsync(...) call and the leading-newline explanation.
  • Converted WriteAnnotationAsync/WriteSkippedAnnotationAsync from async Task to returning the helper's Task directly (callers retain .ConfigureAwait(false)).
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.csDeduplicates the GetTestName call and factors the shared annotation-display logic into a new DisplayAnnotationLineAsync helper; both write methods now return the helper's task directly.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Medium

@github-actionsgithub-actionsBot 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.

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.

✅ 22/22 dimensions clean — no findings.

Clean mechanical refactoring: the testName hoist is safe (GetTestName is a static delegate with no side effects), and the async removal is correct since all callers await inside a try/catch — synchronous propagation from the non-async path reaches the same handler as a faulted Task would.

@Evangelink
Amaury Levé (Evangelink) enabled auto-merge (squash) July 7, 2026 07:38
…inistic
- Address review feedback: revert the hoisted 'testName' local so GetTestName
stays lazy at the two annotation call sites, avoiding an eager property-bag
walk + string allocation on the common passing/in-progress path (keeps the
DisplayAnnotationLineAsync extraction).
- Fix flaky Windows Release CI: GetErrorAnnotation source-location tests relied
on a real throw whose PDB-derived line shifts under Release JIT (net462 vs
net472), producing line=110 vs expected 114. Build a deterministic synthetic
stack frame via [CallerFilePath] instead, keeping both file and line stable
while still exercising the resolver's repo-root/'/_/' relativization.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

# Conflicts:
#	src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.cs
CopilotAI review requested due to automatic review settings July 7, 2026 08:40

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.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9677

GradeTestNotes
A (90–100)GitHubActionsAnnotationReporterTests.
GetSkippedAnnotation_
FallsBackToDefaultReason_
WhenNoExplanation
Clear AAA, exact equality assertion validates the complete output string including percent-encoding and the precise fallback message; no issues found.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. 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. · 89.2 AIC · ⌖ 7.06 AIC · ⊞ 9.5K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit c6ad50d into mainJul 7, 2026
39 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/silver-garbanzo branch July 7, 2026 11:18
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.

[code-simplifier] simplify: extract testName variable and DisplayAnnotationLineAsync helper in GitHubActionsAnnotationReporter

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 > 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

Simplify GitHubActionsAnnotationReporter - #9677

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo
Jul 7, 2026
Merged

Simplify GitHubActionsAnnotationReporter#9677
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9658.

Two small, purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (added in #9641):

  1. Eliminated the duplicate GetTestName call in ConsumeAsync — extracted a testName local computed once before the failure/skip branch.
  2. Extracted a shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, which both ended with an identical _outputDisplay.DisplayAsync(...) call plus the same leading-newline explanation. The helper centralises the call and its full explanation; both write methods now return the helper's Task directly (no longer async).

No functional changes.

Testing

  • Microsoft.Testing.Extensions.GitHubActionsReport builds with 0 warnings / 0 errors across all target frameworks.
  • All Microsoft.Testing.Extensions.UnitTests (including GitHubActionsAnnotationReporterTests) pass unchanged on net8.0, net9.0, net462, and net472.

Extract testName local in ConsumeAsync to avoid the duplicate GetTestName call, and extract the shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, consolidating the leading-newline explanation in one place.
Fixes#9658
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 7, 2026 04:32
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jul 7, 2026

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

This PR applies two purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (originally added in #9641). It removes a duplicated GetTestName call in ConsumeAsync and extracts the identical output-display logic (plus its explanatory comment) shared by the failure and skip write paths into a single DisplayAnnotationLineAsync helper. The extension is a Microsoft.Testing.Platform reporter that emits GitHub Actions ::error/::warning workflow-command annotations, and these changes reduce duplication without altering the emitted annotations.

Changes:

  • Extracted a testName local in ConsumeAsync, computed once instead of per branch.
  • Added a private DisplayAnnotationLineAsync helper that centralizes the _outputDisplay.DisplayAsync(...) call and the leading-newline explanation.
  • Converted WriteAnnotationAsync/WriteSkippedAnnotationAsync from async Task to returning the helper's Task directly (callers retain .ConfigureAwait(false)).
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.csDeduplicates the GetTestName call and factors the shared annotation-display logic into a new DisplayAnnotationLineAsync helper; both write methods now return the helper's task directly.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Medium

@github-actionsgithub-actionsBot 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.

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.

✅ 22/22 dimensions clean — no findings.

Clean mechanical refactoring: the testName hoist is safe (GetTestName is a static delegate with no side effects), and the async removal is correct since all callers await inside a try/catch — synchronous propagation from the non-async path reaches the same handler as a faulted Task would.

@Evangelink
Amaury Levé (Evangelink) enabled auto-merge (squash) July 7, 2026 07:38
…inistic
- Address review feedback: revert the hoisted 'testName' local so GetTestName
stays lazy at the two annotation call sites, avoiding an eager property-bag
walk + string allocation on the common passing/in-progress path (keeps the
DisplayAnnotationLineAsync extraction).
- Fix flaky Windows Release CI: GetErrorAnnotation source-location tests relied
on a real throw whose PDB-derived line shifts under Release JIT (net462 vs
net472), producing line=110 vs expected 114. Build a deterministic synthetic
stack frame via [CallerFilePath] instead, keeping both file and line stable
while still exercising the resolver's repo-root/'/_/' relativization.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

# Conflicts:
#	src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.cs
CopilotAI review requested due to automatic review settings July 7, 2026 08:40

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.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9677

GradeTestNotes
A (90–100)GitHubActionsAnnotationReporterTests.
GetSkippedAnnotation_
FallsBackToDefaultReason_
WhenNoExplanation
Clear AAA, exact equality assertion validates the complete output string including percent-encoding and the precise fallback message; no issues found.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. 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. · 89.2 AIC · ⌖ 7.06 AIC · ⊞ 9.5K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit c6ad50d into mainJul 7, 2026
39 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/silver-garbanzo branch July 7, 2026 11:18
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.

[code-simplifier] simplify: extract testName variable and DisplayAnnotationLineAsync helper in GitHubActionsAnnotationReporter

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

Simplify GitHubActionsAnnotationReporter - #9677

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo
Jul 7, 2026
Merged

Simplify GitHubActionsAnnotationReporter#9677
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9658.

Two small, purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (added in #9641):

  1. Eliminated the duplicate GetTestName call in ConsumeAsync — extracted a testName local computed once before the failure/skip branch.
  2. Extracted a shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, which both ended with an identical _outputDisplay.DisplayAsync(...) call plus the same leading-newline explanation. The helper centralises the call and its full explanation; both write methods now return the helper's Task directly (no longer async).

No functional changes.

Testing

  • Microsoft.Testing.Extensions.GitHubActionsReport builds with 0 warnings / 0 errors across all target frameworks.
  • All Microsoft.Testing.Extensions.UnitTests (including GitHubActionsAnnotationReporterTests) pass unchanged on net8.0, net9.0, net462, and net472.

Extract testName local in ConsumeAsync to avoid the duplicate GetTestName call, and extract the shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, consolidating the leading-newline explanation in one place.
Fixes#9658
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 7, 2026 04:32
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jul 7, 2026

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

This PR applies two purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (originally added in #9641). It removes a duplicated GetTestName call in ConsumeAsync and extracts the identical output-display logic (plus its explanatory comment) shared by the failure and skip write paths into a single DisplayAnnotationLineAsync helper. The extension is a Microsoft.Testing.Platform reporter that emits GitHub Actions ::error/::warning workflow-command annotations, and these changes reduce duplication without altering the emitted annotations.

Changes:

  • Extracted a testName local in ConsumeAsync, computed once instead of per branch.
  • Added a private DisplayAnnotationLineAsync helper that centralizes the _outputDisplay.DisplayAsync(...) call and the leading-newline explanation.
  • Converted WriteAnnotationAsync/WriteSkippedAnnotationAsync from async Task to returning the helper's Task directly (callers retain .ConfigureAwait(false)).
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.csDeduplicates the GetTestName call and factors the shared annotation-display logic into a new DisplayAnnotationLineAsync helper; both write methods now return the helper's task directly.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Medium

@github-actionsgithub-actionsBot 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.

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.

✅ 22/22 dimensions clean — no findings.

Clean mechanical refactoring: the testName hoist is safe (GetTestName is a static delegate with no side effects), and the async removal is correct since all callers await inside a try/catch — synchronous propagation from the non-async path reaches the same handler as a faulted Task would.

@Evangelink
Amaury Levé (Evangelink) enabled auto-merge (squash) July 7, 2026 07:38
…inistic
- Address review feedback: revert the hoisted 'testName' local so GetTestName
stays lazy at the two annotation call sites, avoiding an eager property-bag
walk + string allocation on the common passing/in-progress path (keeps the
DisplayAnnotationLineAsync extraction).
- Fix flaky Windows Release CI: GetErrorAnnotation source-location tests relied
on a real throw whose PDB-derived line shifts under Release JIT (net462 vs
net472), producing line=110 vs expected 114. Build a deterministic synthetic
stack frame via [CallerFilePath] instead, keeping both file and line stable
while still exercising the resolver's repo-root/'/_/' relativization.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

# Conflicts:
#	src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.cs
CopilotAI review requested due to automatic review settings July 7, 2026 08:40

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.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9677

GradeTestNotes
A (90–100)GitHubActionsAnnotationReporterTests.
GetSkippedAnnotation_
FallsBackToDefaultReason_
WhenNoExplanation
Clear AAA, exact equality assertion validates the complete output string including percent-encoding and the precise fallback message; no issues found.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. 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. · 89.2 AIC · ⌖ 7.06 AIC · ⊞ 9.5K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit c6ad50d into mainJul 7, 2026
39 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/silver-garbanzo branch July 7, 2026 11:18
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.

[code-simplifier] simplify: extract testName variable and DisplayAnnotationLineAsync helper in GitHubActionsAnnotationReporter

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

Simplify GitHubActionsAnnotationReporter - #9677

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo
Jul 7, 2026
Merged

Simplify GitHubActionsAnnotationReporter#9677
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9658.

Two small, purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (added in #9641):

  1. Eliminated the duplicate GetTestName call in ConsumeAsync — extracted a testName local computed once before the failure/skip branch.
  2. Extracted a shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, which both ended with an identical _outputDisplay.DisplayAsync(...) call plus the same leading-newline explanation. The helper centralises the call and its full explanation; both write methods now return the helper's Task directly (no longer async).

No functional changes.

Testing

  • Microsoft.Testing.Extensions.GitHubActionsReport builds with 0 warnings / 0 errors across all target frameworks.
  • All Microsoft.Testing.Extensions.UnitTests (including GitHubActionsAnnotationReporterTests) pass unchanged on net8.0, net9.0, net462, and net472.

Extract testName local in ConsumeAsync to avoid the duplicate GetTestName call, and extract the shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, consolidating the leading-newline explanation in one place.
Fixes#9658
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 7, 2026 04:32
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jul 7, 2026

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

This PR applies two purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (originally added in #9641). It removes a duplicated GetTestName call in ConsumeAsync and extracts the identical output-display logic (plus its explanatory comment) shared by the failure and skip write paths into a single DisplayAnnotationLineAsync helper. The extension is a Microsoft.Testing.Platform reporter that emits GitHub Actions ::error/::warning workflow-command annotations, and these changes reduce duplication without altering the emitted annotations.

Changes:

  • Extracted a testName local in ConsumeAsync, computed once instead of per branch.
  • Added a private DisplayAnnotationLineAsync helper that centralizes the _outputDisplay.DisplayAsync(...) call and the leading-newline explanation.
  • Converted WriteAnnotationAsync/WriteSkippedAnnotationAsync from async Task to returning the helper's Task directly (callers retain .ConfigureAwait(false)).
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.csDeduplicates the GetTestName call and factors the shared annotation-display logic into a new DisplayAnnotationLineAsync helper; both write methods now return the helper's task directly.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Medium

@github-actionsgithub-actionsBot 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.

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.

✅ 22/22 dimensions clean — no findings.

Clean mechanical refactoring: the testName hoist is safe (GetTestName is a static delegate with no side effects), and the async removal is correct since all callers await inside a try/catch — synchronous propagation from the non-async path reaches the same handler as a faulted Task would.

@Evangelink
Amaury Levé (Evangelink) enabled auto-merge (squash) July 7, 2026 07:38
…inistic
- Address review feedback: revert the hoisted 'testName' local so GetTestName
stays lazy at the two annotation call sites, avoiding an eager property-bag
walk + string allocation on the common passing/in-progress path (keeps the
DisplayAnnotationLineAsync extraction).
- Fix flaky Windows Release CI: GetErrorAnnotation source-location tests relied
on a real throw whose PDB-derived line shifts under Release JIT (net462 vs
net472), producing line=110 vs expected 114. Build a deterministic synthetic
stack frame via [CallerFilePath] instead, keeping both file and line stable
while still exercising the resolver's repo-root/'/_/' relativization.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

# Conflicts:
#	src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.cs
CopilotAI review requested due to automatic review settings July 7, 2026 08:40

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.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9677

GradeTestNotes
A (90–100)GitHubActionsAnnotationReporterTests.
GetSkippedAnnotation_
FallsBackToDefaultReason_
WhenNoExplanation
Clear AAA, exact equality assertion validates the complete output string including percent-encoding and the precise fallback message; no issues found.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. 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. · 89.2 AIC · ⌖ 7.06 AIC · ⊞ 9.5K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit c6ad50d into mainJul 7, 2026
39 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/silver-garbanzo branch July 7, 2026 11:18
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.

[code-simplifier] simplify: extract testName variable and DisplayAnnotationLineAsync helper in GitHubActionsAnnotationReporter

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

Simplify GitHubActionsAnnotationReporter - #9677

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo
Jul 7, 2026
Merged

Simplify GitHubActionsAnnotationReporter#9677
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9658.

Two small, purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (added in #9641):

  1. Eliminated the duplicate GetTestName call in ConsumeAsync — extracted a testName local computed once before the failure/skip branch.
  2. Extracted a shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, which both ended with an identical _outputDisplay.DisplayAsync(...) call plus the same leading-newline explanation. The helper centralises the call and its full explanation; both write methods now return the helper's Task directly (no longer async).

No functional changes.

Testing

  • Microsoft.Testing.Extensions.GitHubActionsReport builds with 0 warnings / 0 errors across all target frameworks.
  • All Microsoft.Testing.Extensions.UnitTests (including GitHubActionsAnnotationReporterTests) pass unchanged on net8.0, net9.0, net462, and net472.

Extract testName local in ConsumeAsync to avoid the duplicate GetTestName call, and extract the shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, consolidating the leading-newline explanation in one place.
Fixes#9658
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 7, 2026 04:32
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jul 7, 2026

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

This PR applies two purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (originally added in #9641). It removes a duplicated GetTestName call in ConsumeAsync and extracts the identical output-display logic (plus its explanatory comment) shared by the failure and skip write paths into a single DisplayAnnotationLineAsync helper. The extension is a Microsoft.Testing.Platform reporter that emits GitHub Actions ::error/::warning workflow-command annotations, and these changes reduce duplication without altering the emitted annotations.

Changes:

  • Extracted a testName local in ConsumeAsync, computed once instead of per branch.
  • Added a private DisplayAnnotationLineAsync helper that centralizes the _outputDisplay.DisplayAsync(...) call and the leading-newline explanation.
  • Converted WriteAnnotationAsync/WriteSkippedAnnotationAsync from async Task to returning the helper's Task directly (callers retain .ConfigureAwait(false)).
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.csDeduplicates the GetTestName call and factors the shared annotation-display logic into a new DisplayAnnotationLineAsync helper; both write methods now return the helper's task directly.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Medium

@github-actionsgithub-actionsBot 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.

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.

✅ 22/22 dimensions clean — no findings.

Clean mechanical refactoring: the testName hoist is safe (GetTestName is a static delegate with no side effects), and the async removal is correct since all callers await inside a try/catch — synchronous propagation from the non-async path reaches the same handler as a faulted Task would.

@Evangelink
Amaury Levé (Evangelink) enabled auto-merge (squash) July 7, 2026 07:38
…inistic
- Address review feedback: revert the hoisted 'testName' local so GetTestName
stays lazy at the two annotation call sites, avoiding an eager property-bag
walk + string allocation on the common passing/in-progress path (keeps the
DisplayAnnotationLineAsync extraction).
- Fix flaky Windows Release CI: GetErrorAnnotation source-location tests relied
on a real throw whose PDB-derived line shifts under Release JIT (net462 vs
net472), producing line=110 vs expected 114. Build a deterministic synthetic
stack frame via [CallerFilePath] instead, keeping both file and line stable
while still exercising the resolver's repo-root/'/_/' relativization.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

# Conflicts:
#	src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.cs
CopilotAI review requested due to automatic review settings July 7, 2026 08:40

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.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9677

GradeTestNotes
A (90–100)GitHubActionsAnnotationReporterTests.
GetSkippedAnnotation_
FallsBackToDefaultReason_
WhenNoExplanation
Clear AAA, exact equality assertion validates the complete output string including percent-encoding and the precise fallback message; no issues found.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. 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. · 89.2 AIC · ⌖ 7.06 AIC · ⊞ 9.5K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit c6ad50d into mainJul 7, 2026
39 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/silver-garbanzo branch July 7, 2026 11:18
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.

[code-simplifier] simplify: extract testName variable and DisplayAnnotationLineAsync helper in GitHubActionsAnnotationReporter

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

Simplify GitHubActionsAnnotationReporter - #9677

Merged
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo
Jul 7, 2026
Merged

Simplify GitHubActionsAnnotationReporter#9677
Amaury Levé (Evangelink) merged 3 commits into
mainfrom
dev/amauryleve/silver-garbanzo

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Fixes#9658.

Two small, purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (added in #9641):

  1. Eliminated the duplicate GetTestName call in ConsumeAsync — extracted a testName local computed once before the failure/skip branch.
  2. Extracted a shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, which both ended with an identical _outputDisplay.DisplayAsync(...) call plus the same leading-newline explanation. The helper centralises the call and its full explanation; both write methods now return the helper's Task directly (no longer async).

No functional changes.

Testing

  • Microsoft.Testing.Extensions.GitHubActionsReport builds with 0 warnings / 0 errors across all target frameworks.
  • All Microsoft.Testing.Extensions.UnitTests (including GitHubActionsAnnotationReporterTests) pass unchanged on net8.0, net9.0, net462, and net472.

Extract testName local in ConsumeAsync to avoid the duplicate GetTestName call, and extract the shared DisplayAnnotationLineAsync helper from WriteAnnotationAsync and WriteSkippedAnnotationAsync, consolidating the leading-newline explanation in one place.
Fixes#9658
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 7, 2026 04:32
@EvangelinkAmaury Levé (Evangelink) added the state/needs-review Awaiting review from the team. label Jul 7, 2026

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

This PR applies two purely mechanical simplifications to GitHubActionsAnnotationReporter.cs (originally added in #9641). It removes a duplicated GetTestName call in ConsumeAsync and extracts the identical output-display logic (plus its explanatory comment) shared by the failure and skip write paths into a single DisplayAnnotationLineAsync helper. The extension is a Microsoft.Testing.Platform reporter that emits GitHub Actions ::error/::warning workflow-command annotations, and these changes reduce duplication without altering the emitted annotations.

Changes:

  • Extracted a testName local in ConsumeAsync, computed once instead of per branch.
  • Added a private DisplayAnnotationLineAsync helper that centralizes the _outputDisplay.DisplayAsync(...) call and the leading-newline explanation.
  • Converted WriteAnnotationAsync/WriteSkippedAnnotationAsync from async Task to returning the helper's Task directly (callers retain .ConfigureAwait(false)).
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.csDeduplicates the GetTestName call and factors the shared annotation-display logic into a new DisplayAnnotationLineAsync helper; both write methods now return the helper's task directly.

Review details

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Medium

@github-actionsgithub-actionsBot 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.

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.

✅ 22/22 dimensions clean — no findings.

Clean mechanical refactoring: the testName hoist is safe (GetTestName is a static delegate with no side effects), and the async removal is correct since all callers await inside a try/catch — synchronous propagation from the non-async path reaches the same handler as a faulted Task would.

@Evangelink
Amaury Levé (Evangelink) enabled auto-merge (squash) July 7, 2026 07:38
…inistic
- Address review feedback: revert the hoisted 'testName' local so GetTestName
stays lazy at the two annotation call sites, avoiding an eager property-bag
walk + string allocation on the common passing/in-progress path (keeps the
DisplayAnnotationLineAsync extraction).
- Fix flaky Windows Release CI: GetErrorAnnotation source-location tests relied
on a real throw whose PDB-derived line shifts under Release JIT (net462 vs
net472), producing line=110 vs expected 114. Build a deterministic synthetic
stack frame via [CallerFilePath] instead, keeping both file and line stable
while still exercising the resolver's repo-root/'/_/' relativization.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

# Conflicts:
#	src/Platform/Microsoft.Testing.Extensions.GitHubActionsReport/GitHubActionsAnnotationReporter.cs
CopilotAI review requested due to automatic review settings July 7, 2026 08:40

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.

Review details

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

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #9677

GradeTestNotes
A (90–100)GitHubActionsAnnotationReporterTests.
GetSkippedAnnotation_
FallsBackToDefaultReason_
WhenNoExplanation
Clear AAA, exact equality assertion validates the complete output string including percent-encoding and the precise fallback message; no issues found.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. 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. · 89.2 AIC · ⌖ 7.06 AIC · ⊞ 9.5K · [◷]( · )

@Evangelink
Amaury Levé (Evangelink) merged commit c6ad50d into mainJul 7, 2026
39 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the dev/amauryleve/silver-garbanzo branch July 7, 2026 11:18
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.

[code-simplifier] simplify: extract testName variable and DisplayAnnotationLineAsync helper in GitHubActionsAnnotationReporter

3 participants

@Evangelink@0101