From 3e494d11805973eb4253ad0bb1cf442fe3a686e3 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 11 Dec 2025 16:25:11 +0100 Subject: [PATCH 01/10] test: enhance error reporting and grouping in test execution inside Github Action This hopefully ease the read of test logs inside github actions --- .github/workflows/test-smokes.yml | 48 +++++++++++++--- tests/github-actions.ts | 95 +++++++++++++++++++++++++++++++ tests/test.ts | 22 +++++++ 3 files changed, 157 insertions(+), 8 deletions(-) create mode 100644 tests/github-actions.ts diff --git a/.github/workflows/test-smokes.yml b/.github/workflows/test-smokes.yml index b50359571d..393117c3ce 100644 --- a/.github/workflows/test-smokes.yml +++ b/.github/workflows/test-smokes.yml @@ -259,14 +259,36 @@ 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 + if [ $status -eq 0 ]; then + echo ">>> No error in this test file" + else + echo "::error file=${file},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 + echo "::endgroup::" done - [ $haserror -eq 0 ] && echo ">>> All tests passed" || exit 1 + if [ $haserror -eq 1 ]; then + echo "::error title=Test Failures::Failed test buckets: ${failed_tests[*]}" + 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 +299,28 @@ 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" - $haserror=$true - } else { + if ($status -eq 0) { Write-Host ">>> No error in this test file" + } else { + Write-Host "::error file=${file},title=Test Bucket Failed::Test bucket ${file} failed with exit code ${status}" + Write-Host ">>> Error found in test file: ${file}" + $haserror=$true + $failed_tests+=$file } + Write-Host "::endgroup::" } if ($haserror) { + Write-Host "::error title=Test Failures::Failed test buckets: $($failed_tests -join ', ')" + 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/tests/github-actions.ts b/tests/github-actions.ts new file mode 100644 index 0000000000..bc9bc5c207 --- /dev/null +++ b/tests/github-actions.ts @@ -0,0 +1,95 @@ +/* + * github-actions.ts + * + * Utilities for GitHub Actions workflow commands + * See: https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions + */ + +export function isGitHubActions(): boolean { + return Deno.env.get("GITHUB_ACTIONS") === "true"; +} + +export function isVerboseMode(): boolean { + // Check if RUNNER_DEBUG is set (GitHub Actions debug mode) + // or if QUARTO_TEST_VERBOSE is explicitly set + return Deno.env.get("RUNNER_DEBUG") === "1" || + Deno.env.get("QUARTO_TEST_VERBOSE") === "true"; +} + +export interface AnnotationProperties { + file?: string; + line?: number; + endLine?: number; + title?: string; +} + +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"); +} + +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)}`); +} + +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(title: string, fn: () => T): T { + startGroup(title); + try { + return fn(); + } finally { + endGroup(); + } +} + +export async function withGroupAsync(title: string, fn: () => Promise): Promise { + startGroup(title); + try { + return await fn(); + } finally { + endGroup(); + } +} diff --git a/tests/test.ts b/tests/test.ts index 562b415246..433fe20c3a 100644 --- a/tests/test.ts +++ b/tests/test.ts @@ -17,6 +17,7 @@ import { runningInCI } from "../src/core/ci-info.ts"; import { relative, fromFileUrl } from "../src/deno_ral/path.ts"; import { quartoConfig } from "../src/core/quarto.ts"; import { isWindows } from "../src/deno_ral/platform.ts"; +import * as gha from "./github-actions.ts"; export interface TestLogConfig { @@ -244,6 +245,10 @@ export function test(test: TestDescriptor) { } }; let lastVerify; + + // Start a group for this test (collapsed by default) + gha.startGroup(testName); + try { try { @@ -269,8 +274,15 @@ export function test(test: TestDescriptor) { await ver.verify(testOutput); } } + + // Test passed - end the group + gha.endGroup(); } catch (ex) { if (!(ex instanceof Error)) throw ex; + + // Test failed - end the group first so error is visible + gha.endGroup(); + const border = "-".repeat(80); const coloredName = userSession ? colors.brightGreen(colors.italic(testName)) @@ -330,6 +342,16 @@ export function test(test: TestDescriptor) { }); }); } + + // Emit GitHub Actions error annotation + gha.error( + `Test failed: ${testName}\nVerify: ${lastVerify ? lastVerify.name : "unknown"}\n${ex.message}`, + { + file: relPath, + title: `Test Failure: ${testName}`, + } + ); + fail(output.join("\n")); } finally { safeRemoveSync(log); From cb7994d1cb2b54c9119229d285d4130ffe61d176 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 11 Dec 2025 17:58:43 +0100 Subject: [PATCH 02/10] test: improve CI logging with GitHub Actions annotations and grouping Enhance smoke test CI logs for better error visibility and reduced browser load: - Add GitHub Actions error annotations with file paths and test names - Implement workflow-level log grouping (collapsed by default) - Gate verbose shell script messages behind RUNNER_DEBUG or QUARTO_TEST_VERBOSE - Fix Linux CI to run all tests in bucket (no fail-fast) and report failures at end - Update Windows CI to match Linux behavior (collect all failures before exiting) --- tests/run-tests.ps1 | 46 ++++++++++++++++++++++++++++++++------------- tests/run-tests.sh | 38 +++++++++++++++++++++++++++---------- tests/test.ts | 9 --------- 3 files changed, 61 insertions(+), 32 deletions(-) diff --git a/tests/run-tests.ps1 b/tests/run-tests.ps1 index 3210fd9284..c2ed38f177 100644 --- 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 index 8427377867..597b67953a 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -9,6 +9,12 @@ while [ -h "$SOURCE" ]; do # resolve $SOURCE until the file is no longer a symli done export SCRIPT_PATH="$( cd -P "$( dirname "$SOURCE" )" >/dev/null 2>&1 && pwd )" +# Check if verbose mode is enabled (GitHub Actions debug mode or explicit flag) +VERBOSE_MODE=false +if [[ "$RUNNER_DEBUG" == "1" ]] || [[ "$QUARTO_TEST_VERBOSE" == "true" ]]; then + VERBOSE_MODE=true +fi + source $SCRIPT_PATH/../package/scripts/common/utils.sh export QUARTO_ROOT="`cd "$SCRIPT_PATH/.." > /dev/null 2>&1 && pwd`" @@ -42,10 +48,14 @@ if [[ -z $QUARTO_TESTS_FORCE_NO_VENV ]] then # Save possible activated virtualenv for later restauration OLD_VIRTUAL_ENV=$VIRTUAL_ENV - echo "> Activating virtualenv from .venv for Python tests in Quarto" + if [[ "$VERBOSE_MODE" == "true" ]]; then + echo "> Activating virtualenv from .venv for Python tests in Quarto" + fi source "${QUARTO_ROOT}/tests/.venv/bin/activate" - echo "> Using Python from $(which python)" - echo "> VIRTUAL_ENV: ${VIRTUAL_ENV}" + if [[ "$VERBOSE_MODE" == "true" ]]; then + echo "> Using Python from $(which python)" + echo "> VIRTUAL_ENV: ${VIRTUAL_ENV}" + fi quarto_venv_activated="true" fi @@ -126,20 +136,28 @@ else SUCCESS=$? fi -if [[ $quarto_venv_activated == "true" ]] +if [[ $quarto_venv_activated == "true" ]] then - echo "> Exiting virtualenv activated for tests" + if [[ "$VERBOSE_MODE" == "true" ]]; then + echo "> Exiting virtualenv activated for tests" + fi deactivate - echo "> Using Python from $(which python)" - echo "> VIRTUAL_ENV: ${VIRTUAL_ENV}" + if [[ "$VERBOSE_MODE" == "true" ]]; then + echo "> Using Python from $(which python)" + echo "> VIRTUAL_ENV: ${VIRTUAL_ENV}" + fi unset quarto_venv_activated fi if [[ -n $OLD_VIRTUAL_ENV ]] then - echo "> Reactivating original virtualenv" + if [[ "$VERBOSE_MODE" == "true" ]]; then + echo "> Reactivating original virtualenv" + fi source $OLD_VIRTUAL_ENV/bin/activate - echo "> Using Python from $(which python)" - echo "> VIRTUAL_ENV: ${VIRTUAL_ENV}" + if [[ "$VERBOSE_MODE" == "true" ]]; then + echo "> Using Python from $(which python)" + echo "> VIRTUAL_ENV: ${VIRTUAL_ENV}" + fi unset OLD_VIRTUAL_ENV fi diff --git a/tests/test.ts b/tests/test.ts index 433fe20c3a..10e82e5c60 100644 --- a/tests/test.ts +++ b/tests/test.ts @@ -246,9 +246,6 @@ export function test(test: TestDescriptor) { }; let lastVerify; - // Start a group for this test (collapsed by default) - gha.startGroup(testName); - try { try { @@ -274,15 +271,9 @@ export function test(test: TestDescriptor) { await ver.verify(testOutput); } } - - // Test passed - end the group - gha.endGroup(); } catch (ex) { if (!(ex instanceof Error)) throw ex; - // Test failed - end the group first so error is visible - gha.endGroup(); - const border = "-".repeat(80); const coloredName = userSession ? colors.brightGreen(colors.italic(testName)) From a5277000f489626517359d5847b230e0126dc356 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 12 Dec 2025 14:17:01 +0100 Subject: [PATCH 03/10] test: add searchable failure markers to CI logs CI annotations show which tests failed but don't help locate the failure in logs with thousands of lines. Users need to manually scroll through output to find the actual error. --- .github/workflows/test-smokes.yml | 4 ++-- tests/test.ts | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test-smokes.yml b/.github/workflows/test-smokes.yml index 393117c3ce..67dd8bd37a 100644 --- a/.github/workflows/test-smokes.yml +++ b/.github/workflows/test-smokes.yml @@ -272,7 +272,7 @@ jobs: if [ $status -eq 0 ]; then echo ">>> No error in this test file" else - echo "::error file=${file},title=Test Bucket Failed::Test bucket ${file} failed with exit code ${status}" + 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") @@ -308,7 +308,7 @@ jobs: if ($status -eq 0) { Write-Host ">>> No error in this test file" } else { - Write-Host "::error file=${file},title=Test Bucket Failed::Test bucket ${file} failed with exit code ${status}" + 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 $failed_tests+=$file diff --git a/tests/test.ts b/tests/test.ts index 10e82e5c60..d53a785f56 100644 --- a/tests/test.ts +++ b/tests/test.ts @@ -310,9 +310,18 @@ export function test(test: TestDescriptor) { : verifyFailed; const logMessages = logOutput(log); + + // Create distinctive failure marker for easy log navigation + // This helps users find the failure when clicking GitHub Actions annotations + const failureMarker = `━━━ TEST FAILURE: ${testName}`; + const coloredFailureMarker = userSession + ? colors.red(colors.bold(failureMarker)) + : failureMarker; + const output: string[] = [ "", "", + coloredFailureMarker, border, coloredName, coloredTestCommand, @@ -338,8 +347,7 @@ export function test(test: TestDescriptor) { gha.error( `Test failed: ${testName}\nVerify: ${lastVerify ? lastVerify.name : "unknown"}\n${ex.message}`, { - file: relPath, - title: `Test Failure: ${testName}`, + title: `Test Failure [${relPath}]: ${testName}`, } ); From a97262d435de58bb3e57c85ed7bb97a479436704 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 12 Dec 2025 15:32:36 +0100 Subject: [PATCH 04/10] test: add temporary failing test to verify CI annotations Need to verify that failure markers and annotation improvements work correctly in CI logs before finalizing changes. --- tests/smoke/test-failure-marker.test.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 tests/smoke/test-failure-marker.test.ts diff --git a/tests/smoke/test-failure-marker.test.ts b/tests/smoke/test-failure-marker.test.ts new file mode 100644 index 0000000000..10418b53be --- /dev/null +++ b/tests/smoke/test-failure-marker.test.ts @@ -0,0 +1,21 @@ +/* + * test-failure-marker.test.ts + * + * Temporary test to verify CI failure annotations and markers work correctly. + * This test intentionally fails to verify the ━━━ TEST FAILURE: marker appears + * in CI logs and annotations can be used to navigate to failures. + * + * TODO: Remove this test file after verifying CI behavior + */ + +import { testQuartoCmd } from "../test.ts"; +import { docs } from "../utils.ts"; +import { noErrors } from "../verify.ts"; + +// Test intentional failure - render will fail because file doesn't exist +// This should show the distinctive ━━━ TEST FAILURE: marker in CI logs +testQuartoCmd( + "render", + [docs("nonexistent-file.qmd"), "--to", "html"], + [noErrors], +); From 46efa90aa0ca58648d3adb684e713e36ed02d747 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Fri, 12 Dec 2025 20:33:53 +0100 Subject: [PATCH 05/10] test: move bucket error annotations outside collapsed groups Bucket-level error annotations were emitted inside ::group::...::endgroup:: blocks, making them invisible when groups are collapsed. Users had to expand each group to see which buckets failed. Move ::endgroup:: before the error check so bucket failure annotations appear outside the collapsed group and are immediately visible. --- .github/workflows/test-smokes.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test-smokes.yml b/.github/workflows/test-smokes.yml index 67dd8bd37a..0c4b90b32f 100644 --- a/.github/workflows/test-smokes.yml +++ b/.github/workflows/test-smokes.yml @@ -269,6 +269,7 @@ jobs: shopt -s globstar && ./run-tests.sh "$file" status=$? set -e + echo "::endgroup::" if [ $status -eq 0 ]; then echo ">>> No error in this test file" else @@ -277,10 +278,8 @@ jobs: haserror=1 failed_tests+=("$file") fi - echo "::endgroup::" done if [ $haserror -eq 1 ]; then - echo "::error title=Test Failures::Failed test buckets: ${failed_tests[*]}" echo ">>> The following test buckets failed:" for failed in "${failed_tests[@]}"; do echo " - $failed" @@ -305,6 +304,7 @@ jobs: Write-Host ">>> ./run-tests.ps1 ${file}" ./run-tests.ps1 $file $status=$LASTEXITCODE + Write-Host "::endgroup::" if ($status -eq 0) { Write-Host ">>> No error in this test file" } else { @@ -313,10 +313,8 @@ jobs: $haserror=$true $failed_tests+=$file } - Write-Host "::endgroup::" } if ($haserror) { - Write-Host "::error title=Test Failures::Failed test buckets: $($failed_tests -join ', ')" Write-Host ">>> The following test buckets failed:" foreach ($failed in $failed_tests) { Write-Host " - $failed" From 5824a753e1e3f78494651e5a51e89597234b134e Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Mon, 15 Dec 2025 10:14:04 +0100 Subject: [PATCH 06/10] Add a marker for the failing tests summary section in both bash and PowerShell scripts. --- .github/workflows/test-smokes.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-smokes.yml b/.github/workflows/test-smokes.yml index 0c4b90b32f..bee6a72777 100644 --- a/.github/workflows/test-smokes.yml +++ b/.github/workflows/test-smokes.yml @@ -280,7 +280,8 @@ jobs: fi done if [ $haserror -eq 1 ]; then - echo ">>> The following test buckets failed:" + echo "---- FAILING TESTS SUMMARY ----" + echo " The following test buckets failed:" for failed in "${failed_tests[@]}"; do echo " - $failed" done @@ -315,7 +316,8 @@ jobs: } } if ($haserror) { - Write-Host ">>> The following test buckets failed:" + Write-Host "---- FAILING TESTS SUMMARY ----" + Write-Host " The following test buckets failed:" foreach ($failed in $failed_tests) { Write-Host " - $failed" } From 1163bd5f37525d842f67dddcf0888a81772f74e2 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Mon, 15 Dec 2025 18:54:24 +0100 Subject: [PATCH 07/10] Don't show echo for working test --- .github/workflows/test-smokes.yml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test-smokes.yml b/.github/workflows/test-smokes.yml index bee6a72777..8320c67c0f 100644 --- a/.github/workflows/test-smokes.yml +++ b/.github/workflows/test-smokes.yml @@ -270,9 +270,7 @@ jobs: status=$? set -e echo "::endgroup::" - if [ $status -eq 0 ]; then - echo ">>> No error in this test file" - else + 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 @@ -306,9 +304,7 @@ jobs: ./run-tests.ps1 $file $status=$LASTEXITCODE Write-Host "::endgroup::" - if ($status -eq 0) { - Write-Host ">>> No error in this test file" - } else { + 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 From b37d0ef61308c29aa75c970f915e4ddd18c8cdd0 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Mon, 15 Dec 2025 20:03:49 +0100 Subject: [PATCH 08/10] Consolidate GitHub Actions helpers into src/tools/github.ts Moves GitHub Actions workflow command helpers from tests/ to src/tools/ so they can be used throughout the codebase to reduce ungrouped log lines. The consolidated helpers include proper escaping for workflow commands. --- src/core/platform.ts | 4 - src/tools/github.ts | 153 ++++++++++++++++----- tests/github-actions.ts | 95 ------------- tests/integration/playwright-tests.test.ts | 11 +- tests/test.ts | 9 -- 5 files changed, 127 insertions(+), 145 deletions(-) delete mode 100644 tests/github-actions.ts diff --git a/src/core/platform.ts b/src/core/platform.ts index 7d48cdff27..85c34070cd 100644 --- 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 index d8ed92649d..f2d0e6e59a 100644 --- 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(title: string, fn: () => T): T { + startGroup(title); + try { + return fn(); + } finally { + endGroup(); + } +} + +export async function withGroupAsync( + title: string, + fn: () => Promise, +): Promise { + startGroup(title); + try { + return await fn(); + } finally { + endGroup(); + } +} + +// Legacy group function for backward compatibility and alia +export async function group( + title: string, + fn: () => Promise, +): Promise { + 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 { 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, -) { - 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(title: string, fn: () => Promise) { - if (!runningInCI()) { - return fn(); - } - console.log(githubActionsWorkflowCommand("group", title)); - try { - return await fn(); - } finally { - console.log(githubActionsWorkflowCommand("endgroup")); - } -} diff --git a/tests/github-actions.ts b/tests/github-actions.ts deleted file mode 100644 index bc9bc5c207..0000000000 --- a/tests/github-actions.ts +++ /dev/null @@ -1,95 +0,0 @@ -/* - * github-actions.ts - * - * Utilities for GitHub Actions workflow commands - * See: https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions - */ - -export function isGitHubActions(): boolean { - return Deno.env.get("GITHUB_ACTIONS") === "true"; -} - -export function isVerboseMode(): boolean { - // Check if RUNNER_DEBUG is set (GitHub Actions debug mode) - // or if QUARTO_TEST_VERBOSE is explicitly set - return Deno.env.get("RUNNER_DEBUG") === "1" || - Deno.env.get("QUARTO_TEST_VERBOSE") === "true"; -} - -export interface AnnotationProperties { - file?: string; - line?: number; - endLine?: number; - title?: string; -} - -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"); -} - -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)}`); -} - -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(title: string, fn: () => T): T { - startGroup(title); - try { - return fn(); - } finally { - endGroup(); - } -} - -export async function withGroupAsync(title: string, fn: () => Promise): Promise { - startGroup(title); - try { - return await fn(); - } finally { - endGroup(); - } -} diff --git a/tests/integration/playwright-tests.test.ts b/tests/integration/playwright-tests.test.ts index 0381420ee4..9aba61ac97 100644 --- 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/test.ts b/tests/test.ts index d53a785f56..758bad7e48 100644 --- a/tests/test.ts +++ b/tests/test.ts @@ -17,7 +17,6 @@ import { runningInCI } from "../src/core/ci-info.ts"; import { relative, fromFileUrl } from "../src/deno_ral/path.ts"; import { quartoConfig } from "../src/core/quarto.ts"; import { isWindows } from "../src/deno_ral/platform.ts"; -import * as gha from "./github-actions.ts"; export interface TestLogConfig { @@ -343,14 +342,6 @@ export function test(test: TestDescriptor) { }); } - // Emit GitHub Actions error annotation - gha.error( - `Test failed: ${testName}\nVerify: ${lastVerify ? lastVerify.name : "unknown"}\n${ex.message}`, - { - title: `Test Failure [${relPath}]: ${testName}`, - } - ); - fail(output.join("\n")); } finally { safeRemoveSync(log); From e47665a09e8734ddff40dab815b42082911686f0 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Mon, 15 Dec 2025 20:05:48 +0100 Subject: [PATCH 09/10] Revert the addition of failing test that was here for testing purposes. --- tests/smoke/test-failure-marker.test.ts | 21 --------------------- 1 file changed, 21 deletions(-) delete mode 100644 tests/smoke/test-failure-marker.test.ts diff --git a/tests/smoke/test-failure-marker.test.ts b/tests/smoke/test-failure-marker.test.ts deleted file mode 100644 index 10418b53be..0000000000 --- a/tests/smoke/test-failure-marker.test.ts +++ /dev/null @@ -1,21 +0,0 @@ -/* - * test-failure-marker.test.ts - * - * Temporary test to verify CI failure annotations and markers work correctly. - * This test intentionally fails to verify the ━━━ TEST FAILURE: marker appears - * in CI logs and annotations can be used to navigate to failures. - * - * TODO: Remove this test file after verifying CI behavior - */ - -import { testQuartoCmd } from "../test.ts"; -import { docs } from "../utils.ts"; -import { noErrors } from "../verify.ts"; - -// Test intentional failure - render will fail because file doesn't exist -// This should show the distinctive ━━━ TEST FAILURE: marker in CI logs -testQuartoCmd( - "render", - [docs("nonexistent-file.qmd"), "--to", "html"], - [noErrors], -); From b87bdbd82dbd93f742a0491bfbf710ba2b19d8fe Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Tue, 16 Dec 2025 16:07:30 +0100 Subject: [PATCH 10/10] Add more grouping around R packages restore --- .github/workflows/test-smokes.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/test-smokes.yml b/.github/workflows/test-smokes.yml index 8320c67c0f..9ddd42e742 100644 --- 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: