test/ci: improve GitHub Actions test logging and error visibility (#13787)

cderv · web-flow · commit febbc40c5627 · 2025-12-16T17:05:26.000+01:00
- Add GitHub Actions error annotations with file paths and test names  
- Add clear, searchable failure markers in CI logs  
- Group logs by bucket and step; ensure bucket failure annotations stay visible when groups are collapsed  
- Reduce noise: hide logs for passing tests and gate verbose output behind RUNNER_DEBUG / QUARTO_TEST_VERBOSE  
- Run all tests on Linux and Windows (no fail-fast) and report all failures at the end  
- Add grouping around R package restore  
- Consolidate GitHub Actions helper utilities into src/tools/github.ts  
- Remove temporary failing test used to validate annotations
diff --git a/.github/workflows/test-smokes.yml b/.github/workflows/test-smokes.yml
@@ -149,15 +149,23 @@ jobs:
       - name: Restore R packages
         working-directory: tests
         run: |
+          cat("::group::Installing renv if needed\n")
           if (!requireNamespace('renv', quietly = TRUE)) install.packages('renv')
+          cat("::endgroup::\n")
+          cat("::group::Restoring R packages from renv.lock\n")
           renv::restore()
+          cat("::endgroup::\n")
+          cat("::group::Installing dev versions of knitr and rmarkdown\n")
           # Install dev versions for our testing
           # Use r-universe to avoid github api calls
           try(install.packages('rmarkdown', repos = c('https://rstudio.r-universe.dev', getOption('repos'))))
           try(install.packages('knitr', repos = c('https://yihui.r-universe.dev', getOption('repos'))))
+          cat("::endgroup::\n")
           if ('${{ inputs.extra-r-packages }}' != '') {
             cat(sprintf("::notice::Running with the following extra R packages for renv: %s\n", "${{ inputs.extra-r-packages }}"))
+            cat("::group::Installing extra R packages\n")
             renv::install(strsplit("${{ inputs.extra-r-packages }}", split = ",")[[1]])
+            cat("::endgroup::\n")
           }
         shell: Rscript {0}
         env:
@@ -259,14 +267,34 @@ jobs:
           QUARTO_LOG_LEVEL: DEBUG
         run: |
           haserror=0
+          failed_tests=()
           readarray -t my_array < <(echo '${{ inputs.buckets }}' | jq -rc '.[]')
-          for file in "${my_array[@]}"; do 
+          for file in "${my_array[@]}"; do
+            echo "::group::Running ${file}"
             echo ">>> ./run-tests.sh ${file}"
-            shopt -s globstar && ./run-tests.sh $file
+            # Run tests without -e so we don't exit on first failure
+            set +e
+            shopt -s globstar && ./run-tests.sh "$file"
             status=$?
-            [ $status -eq 0 ] && echo ">>> No error in this test file" || haserror=1
+            set -e
+            echo "::endgroup::"
+            if [ $status -ne 0 ]; then
+              echo "::error title=Test Bucket Failed::Test bucket ${file} failed with exit code ${status}"
+              echo ">>> Error found in test file: ${file}"
+              haserror=1
+              failed_tests+=("$file")
+            fi
           done
-          [ $haserror -eq 0 ] && echo ">>> All tests passed" || exit 1
+          if [ $haserror -eq 1 ]; then
+            echo "---- FAILING TESTS SUMMARY ----"
+            echo "    The following test buckets failed:"
+            for failed in "${failed_tests[@]}"; do
+              echo "    - $failed"
+            done
+            exit 1
+          else
+            echo ">>> All tests passed"
+          fi
         working-directory: tests
         shell: bash
 
@@ -277,18 +305,26 @@ jobs:
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
           $haserror=$false
+          $failed_tests=@()
           foreach ($file in ('${{ inputs.buckets }}' | ConvertFrom-Json)) {
+            Write-Host "::group::Running ${file}"
             Write-Host ">>> ./run-tests.ps1 ${file}"
             ./run-tests.ps1 $file
             $status=$LASTEXITCODE
-            if ($status -eq 1) {
-              Write-Host ">>> Error found in test file"
+            Write-Host "::endgroup::"
+            if ($status -ne 0) {
+              Write-Host "::error title=Test Bucket Failed::Test bucket ${file} failed with exit code ${status}"
+              Write-Host ">>> Error found in test file: ${file}"
               $haserror=$true
-            } else {
-              Write-Host ">>> No error in this test file"
+              $failed_tests+=$file
             }
           }
           if ($haserror) {
+            Write-Host "---- FAILING TESTS SUMMARY ----"
+            Write-Host "    The following test buckets failed:"
+            foreach ($failed in $failed_tests) {
+              Write-Host "    - $failed"
+            }
             Exit 1
           } else {
             Write-Host ">>> All tests have passed"
diff --git a/src/core/platform.ts b/src/core/platform.ts
@@ -94,10 +94,6 @@ export function isInteractiveSession() {
   return isRStudio() || isInteractiveTerminal() || isVSCodeOutputChannel();
 }
 
-export function isGithubAction() {
-  return Deno.env.get("GITHUB_ACTIONS") === "true";
-}
-
 export function nullDevice() {
   return isWindows ? "NUL" : "/dev/null";
 }
diff --git a/src/tools/github.ts b/src/tools/github.ts
@@ -4,11 +4,128 @@
  * Copyright (C) 2020-2022 Posit Software, PBC
  */
 
-import { runningInCI } from "../core/ci-info.ts";
 import { GitHubRelease } from "./types.ts";
 
 // deno-lint-ignore-file camelcase
 
+// GitHub Actions Detection
+export function isGitHubActions(): boolean {
+  return Deno.env.get("GITHUB_ACTIONS") === "true";
+}
+
+export function isVerboseMode(): boolean {
+  return Deno.env.get("RUNNER_DEBUG") === "1" ||
+    Deno.env.get("QUARTO_TEST_VERBOSE") === "true";
+}
+
+// GitHub Actions Workflow Command Escaping
+// See: https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions
+export function escapeData(s: string): string {
+  return s
+    .replace(/%/g, "%25")
+    .replace(/\r/g, "%0D")
+    .replace(/\n/g, "%0A");
+}
+
+export function escapeProperty(s: string): string {
+  return s
+    .replace(/%/g, "%25")
+    .replace(/\r/g, "%0D")
+    .replace(/\n/g, "%0A")
+    .replace(/:/g, "%3A")
+    .replace(/,/g, "%2C");
+}
+
+// GitHub Actions Annotations
+export interface AnnotationProperties {
+  file?: string;
+  line?: number;
+  endLine?: number;
+  title?: string;
+}
+
+function formatProperties(props: AnnotationProperties): string {
+  const parts: string[] = [];
+  if (props.file !== undefined) {
+    parts.push(`file=${escapeProperty(props.file)}`);
+  }
+  if (props.line !== undefined) parts.push(`line=${props.line}`);
+  if (props.endLine !== undefined) parts.push(`endLine=${props.endLine}`);
+  if (props.title !== undefined) {
+    parts.push(`title=${escapeProperty(props.title)}`);
+  }
+  return parts.length > 0 ? " " + parts.join(",") : "";
+}
+
+export function error(
+  message: string,
+  properties?: AnnotationProperties,
+): void {
+  if (!isGitHubActions()) return;
+  const props = properties ? formatProperties(properties) : "";
+  console.log(`::error${props}::${escapeData(message)}`);
+}
+
+export function warning(
+  message: string,
+  properties?: AnnotationProperties,
+): void {
+  if (!isGitHubActions()) return;
+  const props = properties ? formatProperties(properties) : "";
+  console.log(`::warning${props}::${escapeData(message)}`);
+}
+
+export function notice(
+  message: string,
+  properties?: AnnotationProperties,
+): void {
+  if (!isGitHubActions()) return;
+  const props = properties ? formatProperties(properties) : "";
+  console.log(`::notice${props}::${escapeData(message)}`);
+}
+
+// GitHub Actions Log Grouping
+export function startGroup(title: string): void {
+  if (!isGitHubActions()) return;
+  console.log(`::group::${escapeData(title)}`);
+}
+
+export function endGroup(): void {
+  if (!isGitHubActions()) return;
+  console.log("::endgroup::");
+}
+
+export function withGroup<T>(title: string, fn: () => T): T {
+  startGroup(title);
+  try {
+    return fn();
+  } finally {
+    endGroup();
+  }
+}
+
+export async function withGroupAsync<T>(
+  title: string,
+  fn: () => Promise<T>,
+): Promise<T> {
+  startGroup(title);
+  try {
+    return await fn();
+  } finally {
+    endGroup();
+  }
+}
+
+// Legacy group function for backward compatibility and alia
+export async function group<T>(
+  title: string,
+  fn: () => Promise<T>,
+): Promise<T> {
+  return await withGroupAsync(title, fn);
+}
+
+// GitHub API
+
 // A Github Release for a Github Repo
 
 // Look up the latest release for a Github Repo
@@ -26,37 +143,3 @@ export async function getLatestRelease(repo: string): Promise<GitHubRelease> {
     return response.json();
   }
 }
-
-// NB we do not escape these here - it's the caller's responsibility to do so
-function githubActionsWorkflowCommand(
-  command: string,
-  value = "",
-  params?: Record<string, string>,
-) {
-  let paramsStr = "";
-  if (params) {
-    paramsStr = " ";
-    let first = false;
-    for (const [key, val] of Object.entries(params)) {
-      if (!first) {
-        first = true;
-      } else {
-        paramsStr += ",";
-      }
-      paramsStr += `${key}=${val}`;
-    }
-  }
-  return `::${command}${paramsStr}::${value}`;
-}
-
-export async function group<T>(title: string, fn: () => Promise<T>) {
-  if (!runningInCI()) {
-    return fn();
-  }
-  console.log(githubActionsWorkflowCommand("group", title));
-  try {
-    return await fn();
-  } finally {
-    console.log(githubActionsWorkflowCommand("endgroup"));
-  }
-}
diff --git a/tests/integration/playwright-tests.test.ts b/tests/integration/playwright-tests.test.ts
@@ -18,6 +18,7 @@ import { fail } from "testing/asserts";
 import { isWindows } from "../../src/deno_ral/platform.ts";
 import { join } from "../../src/deno_ral/path.ts";
 import { existsSync } from "../../src/deno_ral/fs.ts";
+import * as gha from "../../src/tools/github.ts";
 
 async function fullInit() {
   await initYamlIntelligenceResourcesFromFilesystem();
@@ -86,9 +87,15 @@ Deno.test({
         cwd: "integration/playwright",
       });
       if (!res.success) {
-        if (Deno.env.get("GITHUB_ACTIONS") && Deno.env.get("GITHUB_REPOSITORY") && Deno.env.get("GITHUB_RUN_ID")) {
+        if (gha.isGitHubActions() && Deno.env.get("GITHUB_REPOSITORY") && Deno.env.get("GITHUB_RUN_ID")) {
           const runUrl = `https://github.com/${Deno.env.get("GITHUB_REPOSITORY")}/actions/runs/${Deno.env.get("GITHUB_RUN_ID")}`;
-          console.log(`::error file=playwright-tests.test.ts, title=Playwright tests::Some tests failed. Download report uploaded as artifact at ${runUrl}`);
+          gha.error(
+            `Some tests failed. Download report uploaded as artifact at ${runUrl}`,
+            {
+              file: "playwright-tests.test.ts",
+              title: "Playwright tests"
+            }
+          );
         }
         fail("Failed tests with playwright. Look at playwright report for more details.")
       }
diff --git a/tests/run-tests.ps1 b/tests/run-tests.ps1
@@ -3,9 +3,14 @@
 # Determine the path to this script (we'll use this to figure out relative positions of other files)
 $SOURCE = $MyInvocation.MyCommand.Path
 
+# Check if verbose mode is enabled (GitHub Actions debug mode or explicit flag)
+$VERBOSE_MODE = $env:RUNNER_DEBUG -eq "1" -or $env:QUARTO_TEST_VERBOSE -eq "true"
+
 # ------ Setting all the paths required
 
-Write-Host "> Setting all the paths required..."
+if ($VERBOSE_MODE) {
+  Write-Host "> Setting all the paths required..."
+}
 
 # Tests folder 
 # e.g quarto-cli/tests folder
@@ -42,8 +47,9 @@ If ( $null -eq $Env:GITHUB_ACTION -and $null -eq $Env:QUARTO_TESTS_NO_CONFIG ) {
 
 # ----- Preparing running tests ------------
 
-
-Write-Host "> Preparing running tests..."
+if ($VERBOSE_MODE) {
+  Write-Host "> Preparing running tests..."
+}
 
 # Exporting some variables with paths as env var required for running quarto
 $Env:QUARTO_ROOT = $QUARTO_ROOT
@@ -146,15 +152,21 @@ If ($null -eq $Env:QUARTO_TESTS_FORCE_NO_VENV -and $null -ne $Env:QUARTO_TESTS_F
 If ($null -eq $Env:QUARTO_TESTS_FORCE_NO_VENV) {
   # Save possible activated virtualenv for later restauration
   $OLD_VIRTUAL_ENV=$VIRTUAL_ENV
-  Write-Host "> Activating virtualenv from .venv for Python tests in Quarto"
+  if ($VERBOSE_MODE) {
+    Write-Host "> Activating virtualenv from .venv for Python tests in Quarto"
+  }
   . $(Join-Path $QUARTO_ROOT "tests" ".venv/Scripts/activate.ps1")
-  Write-Host "> Using Python from " -NoNewline; Write-Host "$((gcm python).Source)" -ForegroundColor Blue;
-  Write-Host "> VIRTUAL_ENV: " -NoNewline; Write-Host "$($env:VIRTUAL_ENV)" -ForegroundColor Blue;
+  if ($VERBOSE_MODE) {
+    Write-Host "> Using Python from " -NoNewline; Write-Host "$((gcm python).Source)" -ForegroundColor Blue;
+    Write-Host "> VIRTUAL_ENV: " -NoNewline; Write-Host "$($env:VIRTUAL_ENV)" -ForegroundColor Blue;
+  }
   $quarto_venv_activated = $true
 }
 
 
-Write-Host "> Running tests with `"$QUARTO_DENO $DENO_ARGS`" "
+if ($VERBOSE_MODE) {
+  Write-Host "> Running tests with `"$QUARTO_DENO $DENO_ARGS`" "
+}
 
 & $QUARTO_DENO $DENO_ARGS
 
@@ -164,17 +176,25 @@ $DENO_EXIT_CODE = $LASTEXITCODE
 # Add Coverage handling
 
 If($quarto_venv_activated) {
-  Write-Host "> Exiting virtualenv activated for tests"
+  if ($VERBOSE_MODE) {
+    Write-Host "> Exiting virtualenv activated for tests"
+  }
   deactivate
-  Write-Host "> Using Python from " -NoNewline; Write-Host "$((gcm python).Source)" -ForegroundColor Blue;
-  Write-Host "> VIRTUAL_ENV: " -NoNewline; Write-Host "$($env:VIRTUAL_ENV)" -ForegroundColor Blue;
+  if ($VERBOSE_MODE) {
+    Write-Host "> Using Python from " -NoNewline; Write-Host "$((gcm python).Source)" -ForegroundColor Blue;
+    Write-Host "> VIRTUAL_ENV: " -NoNewline; Write-Host "$($env:VIRTUAL_ENV)" -ForegroundColor Blue;
+  }
   Remove-Variable quarto_venv_activated
 }
 If($null -ne $OLD_VIRTUAL_ENV) {
-  Write-Host "> Reactivating original virtualenv"
+  if ($VERBOSE_MODE) {
+    Write-Host "> Reactivating original virtualenv"
+  }
   . "$OLD_VIRTUAL_ENV/Scripts/activate.ps1"
-  Write-Host "> New Python from " -NoNewline; Write-Host "$((gcm python).Source)" -ForegroundColor Blue;
-  Write-Host "> VIRTUAL_ENV: " -NoNewline; Write-Host "$($env:VIRTUAL_ENV)" -ForegroundColor Blue;
+  if ($VERBOSE_MODE) {
+    Write-Host "> New Python from " -NoNewline; Write-Host "$((gcm python).Source)" -ForegroundColor Blue;
+    Write-Host "> VIRTUAL_ENV: " -NoNewline; Write-Host "$($env:VIRTUAL_ENV)" -ForegroundColor Blue;
+  }
   Remove-Variable OLD_VIRTUAL_ENV
 }
 
diff --git a/tests/run-tests.sh b/tests/run-tests.sh
diff --git a/tests/test.ts b/tests/test.ts

Original file line number	Diff line number	Diff line change
`@@ -94,10 +94,6 @@ export function isInteractiveSession() {`
`94`	`94`	`return isRStudio() \|\| isInteractiveTerminal() \|\| isVSCodeOutputChannel();`
`95`	`95`	`}`
`96`	`96`
`97`		`-export function isGithubAction() {`
`98`		`- return Deno.env.get("GITHUB_ACTIONS") === "true";`
`99`		`-}`
`100`		`-`
`101`	`97`	`export function nullDevice() {`
`102`	`98`	`return isWindows ? "NUL" : "/dev/null";`
`103`	`99`	`}`