ci: target compile/test to touched .ql files, fix silent test-step stderr

.github/scripts/pr-compile.sh

@@ -3,48 +3,96 @@ set -euo pipefail
 
 PR_NUMBER=${1:-}
 LANGUAGE=${2}
-# to stop recompiling all queries if multiple files are modified
-LIBRARY_SCANNED=false
 
-echo "[+] Compiling all queries in $LANGUAGE"
-codeql query compile --threads=0 --check-only "./$LANGUAGE/"
+run_lenient_full_compile() {
+    # No --warnings=error: today's tree has pre-existing deprecated-API warnings in a
+    # handful of files that were never caught because no PR-mode diff has ever touched
+    # all of them at once. Failing on that backlog would block CI-infra validation
+    # (workflow_dispatch, or a PR that only touches .github/**) on unrelated debt.
+    echo "[+] Compiling all queries in $LANGUAGE"
+    codeql query compile --threads=0 --check-only "./$LANGUAGE/"
+}
+
+run_strict_full_compile() {
+    echo "[+] Compiling all queries in $LANGUAGE (strict)"
+    codeql query compile --threads=0 --check-only --warnings=error "./$LANGUAGE/"
+}
 
 if [[ -z "$PR_NUMBER" ]]; then
     # No PR context (e.g. workflow_dispatch run directly on a branch) - there is no PR
-    # file list to walk. The plain compile above already covers every query in the
-    # language directory, matching what publish.yml itself requires to ship a pack
-    # (`codeql pack install`/`publish` - neither treats warnings as fatal). We deliberately
-    # do NOT re-run with --warnings=error here: today's tree has pre-existing deprecated-API
-    # warnings in a handful of files that were never caught because no PR-mode diff has ever
-    # touched all of them at once. Failing full-mode runs on that backlog would block CI-infra
-    # validation on unrelated debt. See the tracking issue for the plan to clean up the
-    # backlog and then make PR-mode itself trigger a full strict compile whenever a PR
-    # touches .codeqlversion or a codeql-pack.lock.yml (a dependency/CLI bump can change
-    # behavior across every query, not just the files literally edited).
+    # file list to walk, so always do the full, lenient compile. This matches what
+    # publish.yml itself requires to ship a pack (`codeql pack install`/`publish` -
+    # neither treats warnings as fatal).
+    run_lenient_full_compile
     echo "[+] No PR number provided - full compile above already covered $LANGUAGE. Done."
     exit 0
 fi
 
-for file in $(gh pr view "$PR_NUMBER" --json files --jq '.files.[].path'); do
-    if [[ ! -f "$file" ]]; then
-        continue
+mapfile -t CHANGED_FILES < <(gh pr view "$PR_NUMBER" --json files --jq '.files.[].path')
+
+# A full compile of every query in $LANGUAGE is required whenever a changed file could
+# plausibly affect the compilation of more than just itself: a shared library (.qll,
+# wherever it lives - not just under lib/), qlpack.yml/lockfile/suite metadata, or a
+# dependency/CLI version bump (.codeqlversion, .release.yml). Only a PR that touches
+# nothing but leaf .ql files in $LANGUAGE gets the fast, targeted path below.
+DEPENDENCY_CHANGED=false
+LANG_FILE_SEEN=false
+WORKFLOW_CHANGED=false
+declare -a TOUCHED_QUERIES=()
+
+for file in "${CHANGED_FILES[@]}"; do
+    if [[ "$file" == ".codeqlversion" || "$file" == ".release.yml" ]]; then
+        echo "[+] $file changed - a dependency/CLI version bump can affect every query"
+        DEPENDENCY_CHANGED=true
+    elif [[ "$file" == "$LANGUAGE"/* ]]; then
+        LANG_FILE_SEEN=true
+        if [[ "$file" == *.ql ]]; then
+            TOUCHED_QUERIES+=("$file")
+        else
+            echo "[+] $file changed - not a leaf .ql file, compiling everything in $LANGUAGE"
+            DEPENDENCY_CHANGED=true
+        fi
+    elif [[ "$file" == .github/* ]]; then
+        # The CI script/workflow that drives this very compile step changed - be safe
+        # and validate the whole language rather than trusting the new logic blindly.
+        WORKFLOW_CHANGED=true
     fi
+done
 
-    # if the file is a query file .ql or .qll
-    if [[ "$file" == $LANGUAGE/**.ql ]]; then
-        echo "[+] Compiling $file (in $LANGUAGE)"
+if [[ "$DEPENDENCY_CHANGED" == true ]]; then
+    run_strict_full_compile
+    echo "[+] Complete"
+    exit 0
+fi
 
-        # compile the query
+if [[ "$WORKFLOW_CHANGED" == true ]]; then
+    run_lenient_full_compile
+    # Also strict-compile any touched .ql files on top, matching what would have run
+    # anyway if only those files (and not .github/**) had changed.
+    for file in "${TOUCHED_QUERIES[@]}"; do
+        if [[ ! -f "$file" ]]; then
+            continue
+        fi
+        echo "[+] Compiling $file (in $LANGUAGE)"
         codeql query compile --threads=0 --check-only --warnings=error "./$file"
+    done
+    echo "[+] Complete"
+    exit 0
+fi
 
-    # if lib folder is modified
-    elif [[ "$file" == $LANGUAGE/lib/* ]] && [[ $LIBRARY_SCANNED == false ]]; then
-        echo "[+] Libray changed, compiling all queries in $LANGUAGE"
-        codeql query compile --threads=0 --check-only --warnings=error "./$LANGUAGE/"
-        # set LIBRARY_SCANNED to true to prevent recompiling
-        LIBRARY_SCANNED=true
+if [[ "$LANG_FILE_SEEN" == false ]]; then
+    echo "[+] No compile-relevant changes for $LANGUAGE. Nothing to do."
+    exit 0
+fi
 
+# Fast path: every changed file under $LANGUAGE/ is a leaf .ql file, so only those need
+# a strict recompile - no need to touch the other untouched queries.
+for file in "${TOUCHED_QUERIES[@]}"; do
+    if [[ ! -f "$file" ]]; then
+        continue
     fi
+    echo "[+] Compiling $file (in $LANGUAGE)"
+    codeql query compile --threads=0 --check-only --warnings=error "./$file"
 done
 
 echo "[+] Complete"

.github/workflows/ci.yml

@@ -35,6 +35,8 @@ jobs:
             src:
               - '${{ matrix.language }}/**'
               - '.github/**'
+              - '.codeqlversion'
+              - '.release.yml'
 
       - name: Setup CodeQL
         if: steps.changes.outputs.src == 'true' || github.event_name == 'workflow_dispatch'
@@ -62,49 +64,120 @@ jobs:
       - name: Test Queries
         if: steps.changes.outputs.src == 'true' || github.event_name == 'workflow_dispatch'
         env:
+          GITHUB_TOKEN: ${{ github.token }}
           RUNNER_TEMP: ${{ runner.temp }}
         shell: python
         run: |
           import os
-          import sys
           import subprocess
+          import sys
           from pathlib import Path
 
           def print_error(fmt, *args):
             print(f"::error::{fmt}", *args)
 
           def print_error_and_fail(fmt, *args):
-            print_error(fmt, args)
+            print_error(fmt, *args)
             sys.exit(1)
 
           runner_temp = os.environ['RUNNER_TEMP']
+          language = "${{ matrix.language }}"
+          pr_number = "${{ github.event.number }}"
+          test_root = Path('${{ github.workspace }}', language, 'test')
+
+          def get_pr_changed_files():
+            # No PR context (e.g. workflow_dispatch run directly on a branch) - there is
+            # no PR file list to walk, so the caller always runs the full test suite.
+            if not pr_number:
+              return None
+            result = subprocess.run(
+              ["gh", "pr", "view", pr_number, "--json", "files", "--jq", ".files.[].path"],
+              capture_output=True, text=True, check=True,
+            )
+            return [line for line in result.stdout.splitlines() if line]
+
+          def find_targeted_qlrefs(changed_files):
+            # If this PR only touches leaf .ql file(s) in `language` (no shared library,
+            # pack metadata/lockfile, or CLI/dependency version-bump changes), return the
+            # .qlref file(s) that test those exact quer(y/ies) - which may be empty if none
+            # of the touched queries have test coverage. Otherwise return None, meaning
+            # "run the full test suite" (some other kind of change could affect any test).
+            touched_queries = []
+            for f in changed_files:
+              if f in (".codeqlversion", ".release.yml") or f.startswith(".github/"):
+                return None
+              if f.startswith(f"{language}/"):
+                if f.endswith(".ql"):
+                  touched_queries.append(f)
+                else:
+                  return None
+            if not touched_queries:
+              return None
+
+            src_prefix = f"{language}/src/"
+            touched_rel = set()
+            for q in touched_queries:
+              if not q.startswith(src_prefix):
+                return None
+              touched_rel.add(q[len(src_prefix):])
+
+            # .qlref files declare the query they test via a `query: <path-relative-to-src>`
+            # line - this is the authoritative src<->test mapping (test folder names are
+            # only a loose, unreliable convention, not a formal one).
+            matches = []
+            for qlref in test_root.rglob("*.qlref"):
+              for line in qlref.read_text().splitlines():
+                line = line.strip()
+                if line.startswith("query:"):
+                  if line.split(":", 1)[1].strip() in touched_rel:
+                    matches.append(qlref)
+                  break
+            return matches
+
+          changed_files = get_pr_changed_files()
+          targeted_qlrefs = find_targeted_qlrefs(changed_files) if changed_files is not None else None
 
-          test_root = Path('${{ github.workspace }}', '${{ matrix.language }}', 'test')
-          print(f"Executing tests found (recursively) in the directory '{test_root}'")
           files_to_close = []
           try:
-            # Runners have 4 cores, so split the tests into 4 "slices", and run one per thread
-            num_slices = 4
-            procs = []
+            if targeted_qlrefs is not None:
+              if not targeted_qlrefs:
+                print(f"[+] No tests reference the changed .ql file(s) in {language} - nothing to run")
+                report_path = os.path.join(runner_temp, language, "test_report_slice_1_of_1.json")
+                os.makedirs(os.path.dirname(report_path), exist_ok=True)
+                Path(report_path).write_text("[]")
+                sys.exit(0)
+
+              print(f"[+] PR only touches leaf .ql file(s) in {language} - running {len(targeted_qlrefs)} targeted test(s) instead of the full suite")
+              slices = [(1, 1, [str(p) for p in targeted_qlrefs])]
+            else:
+              print(f"Executing tests found (recursively) in the directory '{test_root}'")
+              # Runners have 4 cores, so split the tests into 4 "slices", and run one per thread
+              num_slices = 4
+              slices = [(n, num_slices, [f"--slice={n}/{num_slices}", str(test_root)]) for n in range(1, num_slices+1)]
 
-            for slice in range(1, num_slices+1):
-              test_report_path = os.path.join(runner_temp, "${{ matrix.language }}", f"test_report_slice_{slice}_of_{num_slices}.json")
+            procs = []
+            for slice_num, total_slices, extra_args in slices:
+              test_report_path = os.path.join(runner_temp, language, f"test_report_slice_{slice_num}_of_{total_slices}.json")
+              test_log_path = os.path.join(runner_temp, language, f"test_log_slice_{slice_num}_of_{total_slices}.txt")
               os.makedirs(os.path.dirname(test_report_path), exist_ok=True)
               test_report_file = open(test_report_path, 'w')
+              test_log_file = open(test_log_path, 'w')
               files_to_close.append(test_report_file)
-              procs.append(subprocess.Popen(["codeql", "test", "run", "--failing-exitcode=122", f"--slice={slice}/{num_slices}", "--ram=2048", "--format=json", test_root], stdout=test_report_file, stderr=subprocess.PIPE))
-
-            for p in procs:
-              _, err = p.communicate()
-              if p.returncode != 0:
-                if p.returncode == 122:
-                  # Failed because a test case failed, so just print the regular output.
-                  # This will allow us to proceed to validate-test-results, which will fail if
-                  # any test cases failed
-                  print(f"{err.decode()}")
-                else:
-                  # Some more serious problem occurred, so print and fail fast
-                  print_error_and_fail(f"Failed to run tests with return code {p.returncode}\n{err.decode()}")
+              files_to_close.append(test_log_file)
+              procs.append((subprocess.Popen(["codeql", "test", "run", "--failing-exitcode=122", "--verbosity=progress", "--ram=2048", "--format=json", *extra_args], stdout=test_report_file, stderr=test_log_file), test_log_path))
+
+            for p, test_log_path in procs:
+              p.wait()
+              # Progress output goes to stderr by default - previously this was only
+              # printed on failure, so a normal passing run looked completely silent.
+              # Always surface it now.
+              log_text = Path(test_log_path).read_text()
+              if log_text:
+                print(log_text)
+              if p.returncode != 0 and p.returncode != 122:
+                # 122 just means a test case failed - validate-test-results will catch
+                # that from the JSON report. Anything else is a real crash - fail fast.
+                print_error_and_fail(f"Failed to run tests with return code {p.returncode}")
           finally:
             for file in files_to_close:
               file.close()
@@ -116,6 +189,7 @@ jobs:
           name: ${{ matrix.language }}-test-results
           path: |
             ${{ runner.temp }}/${{ matrix.language }}/test_report_slice_*.json
+            ${{ runner.temp }}/${{ matrix.language }}/test_log_slice_*.txt
           if-no-files-found: error
 
       - name: Compile / Check Suites & Packs