Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 99 additions & 7 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ jobs:
# against the default branch on non-default branch pushes, which would
# match every accumulated change and defeat the filter.
base: ${{ github.event_name == 'push' && github.event.before || '' }}
# Emit the matched file list so the fast-path step can NAME what it classified
# as documentation. A fast path that silently under-builds is the failure mode
# worth guarding against, so the reason is always printed, never inferred.
list-files: shell
filters: |
# A change to a root build file (the solution, restore config, or THIS workflow) can
# affect every product, so it forces a full build/test/publish.
Expand Down Expand Up @@ -73,24 +77,94 @@ jobs:
darling:
- 'Darling/**'
- '!**/*.md'
# True when any non-documentation file changed. Documentation-only
# changes (e.g. a CHANGELOG or README edit) skip the build/test/
# publish steps below — there is nothing to compile. The job still
# runs so the required 'build' check reports a result.
# The DOCUMENTATION allowlist: files that cannot affect a build under any
# job in this workflow. Deliberately an allowlist of non-executable content,
# not a "everything that isn't code" subtraction — a new file type defaults
# to being treated as code, which is the safe direction to be wrong in.
#
# NOT here, on purpose: *.sql (the installer and sql-validation compile it),
# *.yml (workflows), *.csproj / *.props / packages.lock.json (build inputs),
# and *.cs regardless of how comment-only the change looks — an XML doc
# comment still recompiles, and the compiler is what proves it still builds.
docs:
- '**/*.md'
- 'LICENSE'
- 'CITATION.cff'
- '.gitignore'
- '.gitattributes'
- 'docs/**'
- 'Screenshots/**'
# True when any file OUTSIDE the documentation allowlist changed. A docs-only
# change skips .NET setup, restore and versioning below — there is nothing to
# compile — while the job itself still runs, so the required 'build' check
# reports a result rather than going missing and blocking the merge.
#
# The exclusions MIRROR the docs filter above; they are the same list stated
# negatively because paths-filter has no "not in that other filter" operator.
# Keep the two in step: a path added to docs: but not here still forces a
# restore, and a path excluded here but absent from docs: makes the fast-path
# log under-report what it classified.
code:
- '**'
- '!**/*.md'
- '!LICENSE'
- '!CITATION.cff'
- '!.gitignore'
- '!.gitattributes'
- '!docs/**'
- '!Screenshots/**'

# Decides the docs fast path ONCE, in one place, and says so out loud. Two guards
# keep it off the paths where a skipped restore would be a real loss:
# release — the filter step does not even run there, and a release must always
# compile and publish from a cold, fully restored tree.
# push — dev/main pushes are the integration signal for what just merged, so
# they restore unconditionally even for a docs-only commit. Cheap
# insurance: this only forces the restore back on, it does not force
# the per-product build/test steps, which stay path-gated as before.
# Everything else (pull_request) is eligible, and engages only when NOTHING outside
# the documentation allowlist changed.
- name: Classify change for the docs fast path
id: fastpath
shell: bash
env:
CODE_CHANGED: ${{ steps.filter.outputs.code }}
DOCS_FILES: ${{ steps.filter.outputs.docs_files }}
run: |
set -euo pipefail

if [ "${{ github.event_name }}" = "release" ]; then
echo "engaged=false" >> "$GITHUB_OUTPUT"
echo "::notice title=Full build::Release event - the docs fast path never applies to a release."
exit 0
fi

if [ "${{ github.event_name }}" = "push" ]; then
echo "engaged=false" >> "$GITHUB_OUTPUT"
echo "::notice title=Full build::Push to '${{ github.ref_name }}' - branch pushes always restore, even for a docs-only commit."
exit 0
fi

if [ "${CODE_CHANGED}" = "false" ]; then
echo "engaged=true" >> "$GITHUB_OUTPUT"
echo "::notice title=DOCS FAST PATH ENGAGED::No file outside the documentation allowlist changed, so .NET setup, restore and versioning are skipped. This job still reports its result."
echo "Documentation files classified in this change:"
for f in ${DOCS_FILES}; do echo " - ${f}"; done
else
echo "engaged=false" >> "$GITHUB_OUTPUT"
echo "::notice title=Full build::At least one file outside the documentation allowlist changed."
fi

- name: Setup .NET 10.0
if: steps.filter.outputs.code != 'false'
if: steps.fastpath.outputs.engaged != 'true'
uses: actions/setup-dotnet@v6
with:
dotnet-version: 10.0.x
cache: true
cache-dependency-path: '**/packages.lock.json'

- name: Restore dependencies
if: steps.filter.outputs.code != 'false'
if: steps.fastpath.outputs.engaged != 'true'
run: |
dotnet restore Lite/PerformanceMonitorLite.csproj --locked-mode
dotnet restore Lite.Tests/Lite.Tests.csproj --locked-mode
Expand Down Expand Up @@ -145,7 +219,7 @@ jobs:
run: dotnet test Darling/Darling.Tests/Darling.Tests.csproj -c Release --no-build --verbosity normal

- name: Get version
if: steps.filter.outputs.code != 'false'
if: steps.fastpath.outputs.engaged != 'true'
id: version
shell: pwsh
run: |
Expand Down Expand Up @@ -438,6 +512,24 @@ jobs:
- '!Darling/**/*.md'
- '.github/workflows/build.yml'

# This job's gate was already correct for documentation — a docs-only change leaves
# 'darling' false and every step below no-ops. What it lacked was SAYING so: a job
# that reports success having quietly run nothing looks identical to one that tested
# everything. Costs one step; buys a log you can point at when asking "did this
# actually get tested?".
- name: Report the Darling PG gate decision
shell: bash
run: |
set -euo pipefail

if [ "${{ github.event_name }}" = "release" ]; then
echo "::notice title=Darling PG tests skipped::Release event - the dev push that produced this commit already ran them."
elif [ "${{ steps.filter.outputs.darling }}" = "true" ]; then
echo "::notice title=Darling PG tests running::Darling code (or this workflow) changed."
else
echo "::notice title=Darling PG tests skipped::No Darling code changed - documentation-only Darling edits do not trigger the TimescaleDB suite."
fi

- name: Setup .NET 10.0
if: steps.filter.outputs.darling == 'true'
uses: actions/setup-dotnet@v6
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- **CI: a documentation change stops paying for a .NET restore** ([#1712]) - non-executable changes now skip .NET setup, restore and versioning in the required `build` job, while the job still RUNS so the check reports and cannot block a merge. The gate is an explicit **allowlist** of non-executable content (`**/*.md`, `LICENSE`, `CITATION.cff`, `.gitignore`, `.gitattributes`, `docs/**`, `Screenshots/**`) rather than a subtraction, so an unfamiliar new file type defaults to being treated as code - the safe direction to be wrong in. `*.sql`, `*.yml`, `*.csproj`/`*.props`/`packages.lock.json` are deliberately excluded from the allowlist because some job compiles or consumes each of them, and `*.cs` is excluded however comment-only a change looks, since the compiler is what proves it still builds. **Two guards against under-building**: the fast path never engages on a `release` event or on a push to `dev`/`main` (those restore unconditionally - it only forces the restore back on, per-product build/test steps stay path-gated exactly as before), and both jobs now emit a `::notice::` naming WHY they took the path they took plus the file list classified as documentation, because a job that reports success having quietly run nothing is indistinguishable from one that tested everything. Measured against the real gap: markdown-only changes were ALREADY skipping every heavy step (PR #1707, pure `.md`, ran `build` in 1m43s), so what this actually fixes is non-markdown documentation - a `LICENSE`, `.gitignore` or screenshot edit previously matched the catch-all and paid a six-project locked-mode restore for nothing. The remaining ~1m45s floor is `actions/checkout` on a Windows runner, not work this gate can remove.
- **CI: the Lite fast / analysis-heavy test split collapses back into one step** ([#1701]) - the split existed because the seven analysis classes rebuilt the full DuckDB schema inside every test and their subset alone cost ~9 CI minutes, so a narrower lite_analysis path gate let non-analysis Lite changes skip it. After [#1693]/[#1694]/[#1698] that subset runs in ~66s on the same runner, so the split stopped earning its second test-host spin-up - and its hand-maintained class-name filters were a standing drift risk (a renamed analysis class would silently fall out of the heavy filter into the fast bucket). One Run Lite tests step now runs the whole suite behind the lite path gate; the unconsumed lite_analysis filter block is gone. The lite gate is a strict superset of the old lite_analysis gate, so nothing that ran before is skipped now.
- **Lite.Tests: the shared DuckDB fixture extends to 20 fast-bucket classes** ([#1698]) - rounds out [#1693]/[#1694]: a full-suite trx profile showed 400s+ of serial weight remaining in fast-bucket classes with the same per-test disease (a fresh DuckDB + full schema build inside every test; per-row single-connection seeding in several). All 20 eligible classes now take the class fixture with a data-only reset per test - including the four appender round-trip classes that previously built one database PER TABLE PER TEST - with the special cases handled rather than papered over: FindingStoreTests' v3->v4 analysis-schema migration test hand-creates the legacy table shape and therefore keeps its own private database file while its 15 siblings share; the two MCP classes keep a test-local temp dir for the ServerManager config directory; the server-time-helper collection trio keeps its collection attribute. Six classes are deliberately NOT converted because each needs a private database by design - ArchiveViewDedupTests (parquet + archive-view rebuilds), MuteRulesSurviveResetTests (ArchiveAllAndResetAsync deletes the database file), DuckDbSchemaTests (tests schema creation itself), and the three TestAlertDataHelper consumers (the helper writes parquet into the archive dir and rebuilds views) - that list is the output of reading every candidate class end to end, not a guess. Measured: total serial work across the suite 925s -> 760s (IndexObjectStats 60.6s -> 20.8s, SystemEventsReader 36.2s -> 13.8s, PerformanceCalendarData off the top-15); the local wall is parallelism-bound on a many-core box, but CI's 2-core runner is serial-bound and gets the elimination nearly 1:1. Full suite run twice consecutively: identical 1494/1494 results, zero warnings.
- **Lite.Tests: seeding is batched - one connection and one transaction per seed helper instead of a connection and an auto-commit per ROW** ([#1694]) - the profiling follow-up [#1693] demanded. With the schema builds gone, a per-test trx profile showed the analysis-heavy filter was bounded by seeding I/O: every seeded row opened a fresh DuckDB connection and auto-committed a single INSERT, measured at ~90ms/row, and AnomalyDetectorTests (whose tests seed 100-200 baseline rows apiece through its private helpers) was the ENTIRE critical path - 154.5s serial against the filter's ~154s wall. Three mechanical changes, no test-body semantics touched: AnomalyDetectorTests and BaselineProviderTests reuse one per-test-instance seed connection for every row, with AnomalyDetectorTests' seven SeedBaseline* loops each inside one BEGIN/COMMIT; TestDataSeeder holds one lazily-opened connection per instance and every seed helper wraps its inserts in a SeedBatch (BEGIN on construct, COMMIT on dispose) so a helper's rows share one WAL flush - except ClearTestDataAsync, deliberately un-batched because its per-table try/catch DELETEs would abort an explicit transaction on the first missing table and silently skip the rest of the clear; and TestDataSeeder is now IDisposable with all 48 construction sites taking using var seeder, which also hands the speedup to the four seeder-using classes outside the heavy filter (DatabaseFilter, FactScorer, FinOps, FindingStore) for free. Measured locally on an idle box: the analysis-heavy filter 164s -> 19s, the FULL suite 233s -> 78s, per-class serial ceilings from 154.5s down to 20s. For the record, #1693's own CI run showed the fixture change alone was net neutral on the runner (612s step vs the 556s reference, with the fast step drifting +10% identically) - the per-row seeding was the bottleneck all along, and this is the change expected to collapse that CI step. Isolation re-proven under the new transaction semantics: batches commit before any read, a mid-batch failure cannot mask the test's real exception, and the full suite ran twice consecutively with identical 1494/1494 results.
Expand Down Expand Up @@ -1690,6 +1691,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#1700]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1700
[#1703]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1703
[#1708]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1708
[#1712]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1712
[#1710]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1710
[#1690]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1690
[#1693]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1693
Expand Down
Loading