Skip to content

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

Merged
felickz merged 2 commits into
mainfrom
ci-targeted-testing
Aug 1, 2026
Merged

ci: target compile/test to touched .ql files, fix silent test-step stderr#199
felickz merged 2 commits into
mainfrom
ci-targeted-testing

Conversation

@felickz

@felickz felickz commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

pr-compile.sh: replace unconditional full-language compile with a
three-way gate. A PR that only touches leaf .ql files now gets a fast,
strict per-file recompile instead of compiling every query in the
language. Any change to a shared library (.qll anywhere, not just
lib/), qlpack.yml/lockfile/suite metadata, or a dependency/CLI version
bump (.codeqlversion, .release.yml) still triggers a full, strict
recompile - this also fixes a latent gap where a deleted dependency
file was silently ignored. .github/** changes get a lenient full
compile (matching the prior no-PR-context behavior) plus a strict
per-file recompile of any touched queries.

ci.yml Test Queries step: mirror the same classification to scope
codeql test run to just the .qlref file(s) that test the touched
.ql file(s), resolved via each .qlref's authoritative query: line
(not folder-name convention, which is unreliable - see CWE-089/
CWE-208 test layouts). Falls back to the full 4-way sliced suite for
any non-fast-path change.

Also fixes test-step stderr always being discarded on success: the
child's progress output now goes to a real per-slice log file that is
always printed after the process exits, instead of being captured via
communicate() and only shown on failure. Upload test results now also
picks up these log files for post-hoc debugging.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com
Copilot-Session: 84006984-666d-4926-bbb8-795d91f3b5ab


Stack created with GitHub Stacks CLIGive Feedback 💬

…derr

pr-compile.sh: replace unconditional full-language compile with a
three-way gate. A PR that only touches leaf .ql files now gets a fast,
strict per-file recompile instead of compiling every query in the
language. Any change to a shared library (.qll anywhere, not just
lib/), qlpack.yml/lockfile/suite metadata, or a dependency/CLI version
bump (.codeqlversion, .release.yml) still triggers a full, strict
recompile - this also fixes a latent gap where a deleted dependency
file was silently ignored. .github/** changes get a lenient full
compile (matching the prior no-PR-context behavior) plus a strict
per-file recompile of any touched queries.

ci.yml Test Queries step: mirror the same classification to scope
codeql test run to just the .qlref file(s) that test the touched
.ql file(s), resolved via each .qlref's authoritative query: line
(not folder-name convention, which is unreliable - see CWE-089/
CWE-208 test layouts). Falls back to the full 4-way sliced suite for
any non-fast-path change.

Also fixes test-step stderr always being discarded on success: the
child's progress output now goes to a real per-slice log file that is
always printed after the process exits, instead of being captured via
communicate() and only shown on failure. Upload test results now also
picks up these log files for post-hoc debugging.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 84006984-666d-4926-bbb8-795d91f3b5ab

Copilot AI 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 optimizes CI for CodeQL pack validation by scoping query compilation and test execution to the specific .ql files changed in a pull request when it’s safe to do so, while preserving full-suite behavior for changes that can affect the broader build/test surface. It also improves observability of codeql test run by always surfacing progress output that previously went missing on successful runs.

Changes:

  • Update .github/scripts/pr-compile.sh to choose between strict full-language compile vs strict per-file compile based on PR file changes.
  • Update the CI “Test Queries” step to run targeted .qlref tests for touched leaf queries (when eligible), otherwise run the full sliced suite.
  • Persist and upload per-slice test stderr/progress logs so successful runs are no longer silent and logs are available for debugging.
Show a summary per file
File Description
.github/workflows/ci.yml Adds targeted test selection via .qlrefquery: mapping, fixes missing stderr/progress visibility, and uploads per-slice logs.
.github/scripts/pr-compile.sh Replaces unconditional full-language compilation with a gated strict full vs strict per-file compile strategy based on the PR diff.

Review details

Suppressed comments (1)

.github/workflows/ci.yml:107

  • This PR adds logic to treat changes to .codeqlversion / .release.yml as “must run full suite”, but the dorny/paths-filter gate for compile-and-test only matches ${{ matrix.language }}/** and .github/**. A PR that only bumps .codeqlversion/.release.yml will skip the entire compile/test job, so these checks never take effect (and CI won’t validate dependency/CLI bumps as described).
            for f in changed_files:
              if f in (".codeqlversion", ".release.yml") or f.startswith(".github/"):
                return None
              if f.startswith(f"{language}/"):
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread .github/scripts/pr-compile.sh
Comment thread .github/workflows/ci.yml
@felickz

felickz commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Live validation: targeted CI on a real single-file PR

To validate this without guessing, I stacked a throwaway PR (#200, closing it now that it's confirmed)
on top of this branch that touches exactly one leaf .ql file
(java/src/security/CWE-089/MyBatisMapperXmlSqlInjection.ql - a QLDoc-only tweak, zero functional
change). Since #200's base is this branch, its diff to GitHub really is just that one file, so it's a
realistic exercise of the new logic end-to-end (paths-filter -> pr-compile.sh -> targeted codeql test run), not a synthetic test.

Before (full-suite java compile-and-test, from a real recent PR that changed one query -
run 30659438886, job 91251853824, commit 600b214):

After (targeted java compile-and-test, same kind of single-query PR, on this branch -
run 30662826840, job 91262805147):

Step Before (full suite) After (targeted) Change
Install Packs (pre-existing step, not touched by this PR) 91s 565s not comparable - registry/network variance between runs, unrelated to this change
Compile Queries 625s (10m 25s) 19s ~33x faster
Test Queries 1,424s (23m 44s) 102s (1m 42s) ~14x faster
Compile + Test combined 2,049s (34m 9s) 121s (2m 1s) ~17x faster
Total job wall time 2,168s (36m 8s) 704s (11m 44s) ~3.1x faster (dominated by the Install Packs variance above, not this change)

The two steps this PR actually changes (Compile Queries, Test Queries) went from 34m 9s -> 2m 1s
for a PR that only touches one query - a ~17x speedup, with test-step progress output now visible in
the log instead of silently discarded on success.

Any PR that touches a shared .qll, qlpack.yml/lockfile, or .codeqlversion/.release.yml still gets
the full, strict, all-queries pass exactly as before - this only fast-paths the common "single query
added/fixed" case.

Closing #200 now - it's served its purpose.

…le bug

The compile-and-test job's dorny/paths-filter only watched
`${{ matrix.language }}/**` and `.github/**`, so a PR that only bumps
.codeqlversion/.release.yml never set steps.changes.outputs.src and
the whole Install Packs/Compile Queries/Test Queries sequence was
skipped for every language on a real pull_request-triggered run - the
DEPENDENCY_CHANGED full-recompile branch this PR adds was unreachable
outside workflow_dispatch. Add both paths to the filter so a
dependency/CLI version bump actually triggers full validation across
every language, as intended.

Also fix print_error_and_fail passing args as an unpacked tuple
instead of *args - it printed a stray '()' after fatal error messages.
Pre-existing bug carried over unchanged from main, but it's inside the
block this PR rewrites, so fixing it here.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 84006984-666d-4926-bbb8-795d91f3b5ab
Copilot AI review requested due to automatic review settings July 31, 2026 20:46

Copilot AI 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: 0 new
  • Review effort level: Lite

@felickz
felickz merged commit 3776027 into main Aug 1, 2026
24 checks passed
@felickz
felickz deleted the ci-targeted-testing branch August 1, 2026 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants