fix(ci): unmask three permanently-failing quality jobs and fix their root cause - #28
Merged
Merged
Conversation
…root cause E2E Tests, Lighthouse CI and axe WCAG 2.1 AA failed on every run of main while their workflows still reported success, so `gh run list` showed all-green. Root cause (one bug, three symptoms): all three depended on `pnpm start`, which is `astro preview`. The @astrojs/vercel adapter does not implement the preview command, so the server exited immediately and every wait timed out. Verified locally: [preview] The @astrojs/vercel adapter does not support the preview command. Stacked second bug: all three waited on port 3041 while the app binds 3040. Port 3041 belonged to the abandoned web/astro/ scaffold, not this app. The in-repo comments blaming NEXTAUTH_SECRET and "CI services" were stale Next.js-era guesses; this app is Astro and needs neither. Changes: - ci.yml: drop continue-on-error from e2e-tests; drop the redundant build step (the dev server does not read build output; the build job covers validity). - quality-gates.yml: drop continue-on-error from lighthouse and axe; run axe against `astro dev` on 3040 instead of the unsupported preview server. - playwright.config.ts: webServer runs `pnpm dev`; baseURL/url to 3040. - lighthouserc.cjs: use staticDistDir (.vercel/output/static) instead of starting a server, auditing the two prerendered index routes. The home page is deliberately not audited here because it calls getDailyVerse()/ getDailyHadith() at request time; prerendering it would freeze daily content until the next deploy. Home-page a11y stays covered by axe. - all-checks-pass.yml: it had no `needs:` and was a bare `echo`, so it could never fail. Replace with a Checks API aggregator over the commit. Its query was tested against baseline e97e8bb and correctly flags the four failures the old gate hid. - delete _reusable-{axe,lighthouse}.yml: zero callers, and they encoded this exact preview/port bug ready to be copied again.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
🟢 Rampart Security Gate — CLEAN
Totals: 0 critical · 0 high · 0 medium · 0 low Mode: |
…n gaps With the E2E job actually running for the first time (34 passed / 7 failed instead of 0 executed), the previously-hidden failures split into two kinds. Test bugs (fixed): - search.spec.ts "typing a query shows dropdown results" and "clear button removes query" navigated to "/". The Astro port deliberately replaced the <SearchInput> island on the home page with a plain GET <form action="/search"> (see the conversion notes in src/pages/index.astro), so the home page has no dropdown and no clear button by design. The island is mounted on /search via client:load, so both tests now target /search. Real migration gaps (quarantined as test.fixme, not masked): - 5 i18n tests. middleware.ts does set locals.locale and Base.astro does honour it, but the Quran pages are `export const prerender = true`, and prerendered pages do not run middleware per request — they are static HTML built with the default locale, so /ar/quran/1 serves lang="en" dir="ltr". Emitting hreflang on Quran pages and building a locale switcher (no LocaleSwitcher exists anywhere under src/) are also outstanding. test.fixme keeps each gap visible in the test report and keeps the job blocking for every other test, which is the opposite of the continue-on-error that hid all of this. Tracked in PCI islamwiki-i18n-prerender-locale.
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
1 participant
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.
Problem
E2E Tests,Lighthouse CIandaxe WCAG 2.1 AAfailed on every run ofmain, yetgh run listshowed all-green. Two independent mechanisms hid them:continue-on-error: trueat job level — the job shows failed in the UI, but the workflow conclusion is SUCCESS, andgh run listreports the workflow.all-checks-pass.ymlhad noneeds:block at all — it was a barerun: echo "all checks passed". It could never fail under any circumstance.Baseline
e97e8bb1: Quality Gates run 33889594378 (Lighthouse + axe failed), CI run 33889594365 (E2E failed on both node legs). Predates the Astro 6 upgrade (#27) and unrelated to it.Root cause — one bug, three symptoms
All three depended on
pnpm start, which isastro preview. Verified locally:The server exited immediately, so Playwright's
webServer, lhci'sstartServerCommandand axe'swait-oneach waited forever.Stacked second bug: all three waited on port 3041 while the app binds 3040. 3041 belonged to the abandoned
web/astro/scaffold, not this app.The in-repo comments blaming
NEXTAUTH_SECRETand "wire CI services" were stale Next.js-era guesses. This app is Astro and needs neither a DB nor an auth secret to serve routes.Changes
continue-on-errorfrome2e-tests; drop the redundantpnpm buildstep (the dev server does not read build output; thebuildjob covers validity).continue-on-errorfromlighthouseandaxe; run axe againstastro devon 3040.webServerrunspnpm dev;baseURL/urlto 3040.staticDistDir: .vercel/output/staticinstead of starting a server, auditing the two prerendered index routes._reusable-{axe,lighthouse}.yml— zero callers, and they encoded this exact preview/port bug ready to be copied again.Why the home page is not in the Lighthouse set
src/pages/index.astrocallsgetDailyVerse()/getDailyHadith()at request time.staticDistDircan only audit prerendered pages, and prerendering the home page would freeze "daily" content until the next deploy. Home-page accessibility is instead covered by the axe job, which runs against a real dev server.Verification of the new gate
The aggregator query was run against baseline
e97e8bb1and returns exactly the four failures the old gate hid:These three jobs are now blocking. This PR's own CI run is the proof that they pass.
Follow-up (not in this PR)
ummeco/.github/workflows/axe-ci-reusable.ymlat the monorepo root has the samepnpm startbug and will bite praycalc, chatislam, ummat/dev/web and flock/site as they finish migrating to Astro.