ci: promote linux-arm64 + alpine-musl to enforcing; close the native-deps deferral - #357
Merged
Conversation
…deferral The `ci-nonblocking-platform-native-deps` deferral's own removal condition — "both non-blocking legs pass the Test step in CI (green)" — is now met, and its premise is obsolete. It was written when libSkiaSharp genuinely would not load on those images (arm64: `undefined symbol: uuid_generate_random`, then FT_Get_BDF_Property; alpine: musl native-load failure). That is fixed: the fontconfig-hardening step plus the apk prerequisites, together with the SkiaSharp 4.150.1 bump (#355), load the native cleanly. The evidence is that both legs were running 8604 of 8609 tests, and their only failures were two font-dependent ASSERTIONS — AutoHeightFlexTimelineFooterTests, fixed in #356 — never a native-load error. Both have since run the full suite green. - linux-arm64 loses `nonblocking: true`; the alpine job loses `continue-on-error: true`. Both are renamed accordingly (the ", non-blocking" suffix is part of the check name). Neither name is in branch protection's required contexts today, so no existing required check breaks. - macos-x64 stays non-blocking and keeps its own deferral: hosted Intel-mac runner availability, which nothing in this repo can fix. - Deferral removed from docs/deferrals.md + DeferralsParityTests (the documented convention for picking one up), and the macos-x64 entry's cross-reference rewritten so it no longer points at a deleted anchor. NOTE for the maintainer: making these legs enforcing turns the WORKFLOW red on failure, but it does not block a merge until the two renamed contexts are added to branch protection's required checks — worth doing alongside `benchmark gate (linux-x64)`, which is still not required. (PROGRESS.md was also rolled, but it is gitignored — untracked deliberately in #284 as an internal doc — so it stays local and out of this PR.) Verified: build 0 errors; UnitTests 8614 passed / 3 skipped (incl. DeferralsParityTests); git diff --check clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Promotes previously non-blocking CI coverage for linux-arm64 and alpine-musl-x64 to enforcing, and removes the now-obsolete deferral documentation/test parity entry, leaving macos-x64 as the only non-blocking leg due to hosted runner availability.
Changes:
- Make
linux-arm64enforcing by removing the matrixnonblocking: trueflag. - Make
alpine-musl-x64enforcing by removingcontinue-on-error: trueand updating the check name. - Remove the
ci-nonblocking-platform-native-depsdeferral entry fromdocs/deferrals.mdand updateDeferralsParityTestsparity expectations accordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/NetPdf.UnitTests/Docs/DeferralsParityTests.cs |
Removes the retired deferral from the expected-ID/priority lists to keep doc/test parity correct. |
docs/deferrals.md |
Deletes the obsolete deferral section and updates the remaining macOS deferral text to remove the old cross-reference. |
.github/workflows/ci.yml |
Promotes linux-arm64 and alpine-musl-x64 from non-blocking to enforcing; updates rationale comments and check names. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PR #357 review [P2] — valid, and it names a gap this PR itself created. Promoting linux-arm64 + alpine made them WORKFLOW-enforcing (a failure turns the run red) but NOT merge-enforcing: `main`'s branch protection still requires only build+test (linux-x64) / (windows-x64) / (macos-arm64), security-gate and dependency-scan — verified against the live API. Deleting the old `ci-nonblocking-platform-native-deps` entry removed the only thing tracking the remaining step, so it could quietly be forgotten. The reviewer offered either applying the setting or keeping a tracked follow-up. Applying it is a repository SETTING that needs admin rights and cannot land through a PR, so this takes the tracked-follow-up option and records it properly: - New deferral `ci-branch-protection-required-contexts` (P2) states the current required list, the three checks missing from it, and the exact `gh api` command — including the trap that the API REPLACES the context list, so all eight must be sent, and that the two renamed checks must not be re-added under their old ", non-blocking" names. - Rated P2, not P3, because the third missing context is `benchmark gate (linux-x64)`: until it is required, a genuine perf regression reports red and still merges, which silently weakens the CLAUDE.md performance contract. - ci.yml now carries a CAVEAT next to "Enforcing matrix" pointing at the deferral, so the distinction is visible where the enforcement is declared. - Registered in DeferralsParityTests (IDs + P2 priority), which is what makes the entry impossible to drop silently. Verified: build 0 errors; UnitTests 8614 passed / 3 skipped; branch-protection contexts re-read from the API to confirm the entry's factual claims. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Picks up the
ci-nonblocking-platform-native-depsdeferral. Its own removal condition — "both non-blocking legs pass the Test step in CI (green)" — is now met, and the premise it was written on is obsolete.Why the premise no longer holds
The deferral described a genuine native-loading failure:
libSkiaSharp.so: undefined symbol: uuid_generate_random, thenFT_Get_BDF_Propertyonce libuuid was preloadedThat is fixed. The fontconfig-hardening step and the
apkprerequisites, together with the SkiaSharp 4.150.1 bump in #355, load the native cleanly.The decisive evidence is what those legs were actually doing while still labelled red: 8604 of 8609 tests passing, with the only two failures being font-dependent assertions in
AutoHeightFlexTimelineFooterTests— never a native-load error. Those were fixed in #356, and both legs have since run the full suite green:linux-arm64alpine-musl-x64Changes
linux-arm64losesnonblocking: true; the alpine job losescontinue-on-error: true. Both are renamed as a result — the", non-blocking"suffix is part of the check name. Neither name is in branch protection's required contexts today, so no existing required check breaks.macos-x64stays non-blocking and keeps its own separate deferral: hosted Intel-mac runner availability, which nothing in this repo can fix. It is now the only non-blocking leg.docs/deferrals.mdandDeferralsParityTests(the documented convention for picking one up), and themacos-x64entry's cross-reference is rewritten so it no longer points at a deleted anchor.ci.ymlreplaced with what is actually true now.Maintainer action still needed
Making these legs enforcing turns the workflow red on failure, but it does not block a merge until the two renamed contexts are added to branch protection's required checks:
build+test (linux-arm64)build+test (alpine-musl-x64)Worth doing alongside
benchmark gate (linux-x64), which is still not a required check — so even now that it measures again (#356), a genuine perf regression would not block a merge.Verification
Build 0 errors · UnitTests 8614 passed / 3 skipped (including
DeferralsParityTests, which enforces the doc/ID parity this PR changes) ·git diff --checkclean.PROGRESS.mdwas rolled too, but it is gitignored — untracked deliberately in #284 as an internal doc — so it stays local and out of this PR.