From 26292748412a972e16da90d939160b9710ad62c0 Mon Sep 17 00:00:00 2001 From: sudacode Date: Sat, 7 Mar 2026 16:38:10 -0800 Subject: [PATCH] Harden renderer CSS selector parsing in stylesheet tests - Split selector lists on top-level commas to handle `:is(...)` safely - Normalize selector whitespace before matching extracted CSS rule blocks - Replace brittle full-file regex checks with targeted hover/selection block assertions - Add TASK-114 backlog record for CI fix on PR #15 --- ...sk-114 - Fix-failing-CI-checks-on-PR-15.md | 62 ++++++++++++ src/renderer/subtitle-render.test.ts | 97 ++++++++++++++++--- 2 files changed, 148 insertions(+), 11 deletions(-) create mode 100644 backlog/tasks/task-114 - Fix-failing-CI-checks-on-PR-15.md diff --git a/backlog/tasks/task-114 - Fix-failing-CI-checks-on-PR-15.md b/backlog/tasks/task-114 - Fix-failing-CI-checks-on-PR-15.md new file mode 100644 index 00000000..9e806d5e --- /dev/null +++ b/backlog/tasks/task-114 - Fix-failing-CI-checks-on-PR-15.md @@ -0,0 +1,62 @@ +--- +id: TASK-114 +title: Fix failing CI checks on PR 15 +status: Done +assignee: + - codex +created_date: '2026-03-08 00:34' +updated_date: '2026-03-08 00:37' +labels: + - ci + - test +dependencies: [] +references: + - src/renderer/subtitle-render.test.ts + - src/renderer/style.css + - .github/workflows/ci.yml +priority: high +--- + +## Description + + +Investigate the failing GitHub Actions CI run for PR #15 on branch `yomitan-fork`, fix the underlying test or code regression, and verify the affected local test/CI lane passes. + + +## Acceptance Criteria + +- [x] #1 Identified the concrete failing CI job and captured the relevant failure context +- [x] #2 Implemented the minimal code or test change needed to resolve the CI failure +- [x] #3 Verified the affected local test target and the broader fast CI test lane pass + + +## Implementation Plan + + +1. Inspect the failing GitHub Actions run and confirm the exact failing test/assertion. +2. Reproduce the failing renderer stylesheet test locally and compare the assertion against current CSS. +3. Apply the minimal test or stylesheet fix needed to restore the intended hover/selection behavior. +4. Re-run the targeted renderer test, then re-run `bun run test` to verify the fast CI lane is green. + + +## Implementation Notes + + +GitHub Actions run 22810400921 failed in job build-test-audit, step `Test suite (source)`, with a single failing test: `JLPT CSS rules use underline-only styling in renderer stylesheet` in src/renderer/subtitle-render.test.ts. + +Reproduced the failing test locally with `bun test src/renderer/subtitle-render.test.ts`. The failure was a brittle stylesheet assertion, not a renderer behavior regression. + +Updated the renderer stylesheet test helper to split selectors safely across `:is(...)` commas and normalize multiline selector whitespace, then switched the failing hover/JLPT assertions to inspect extracted rule blocks instead of matching the entire CSS file text. + +Verification passed with `bun test src/renderer/subtitle-render.test.ts` and `bun run test`. + + +## Final Summary + + +Investigated GitHub Actions CI run `22810400921` for PR #15 and confirmed the only failing job was `build-test-audit`, step `Test suite (source)`, with a single failure in `src/renderer/subtitle-render.test.ts` (`JLPT CSS rules use underline-only styling in renderer stylesheet`). + +The renderer CSS itself was still correct; the regression was in the test helper. `extractClassBlock` was splitting selector lists on every comma, which breaks selectors containing `:is(...)`, and the affected assertions fell back to brittle whole-file regex matching against a multiline selector. Fixed the test by teaching the helper to split selectors only at top-level commas, normalizing selector whitespace around multiline `:not(...)` / `:is(...)` clauses, and asserting on extracted rule blocks for the plain-word hover and JLPT-only hover/selection rules. + +Verification: `bun test src/renderer/subtitle-render.test.ts` passed, and `bun run test` passed end to end (the same fast lane that failed in CI). + diff --git a/src/renderer/subtitle-render.test.ts b/src/renderer/subtitle-render.test.ts index fda44433..334d0ee4 100644 --- a/src/renderer/subtitle-render.test.ts +++ b/src/renderer/subtitle-render.test.ts @@ -138,17 +138,15 @@ function extractClassBlock(cssText: string, selector: string): string { const ruleRegex = /([^{}]+)\{([^}]*)\}/g; let match: RegExpExecArray | null = null; let fallbackBlock = ''; + const normalizedSelector = normalizeCssSelector(selector); while ((match = ruleRegex.exec(cssText)) !== null) { const selectorsBlock = match[1]?.trim() ?? ''; const selectorBlock = match[2] ?? ''; - const selectors = selectorsBlock - .split(',') - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0); + const selectors = splitCssSelectors(selectorsBlock); - if (selectors.includes(selector)) { + if (selectors.some((entry) => normalizeCssSelector(entry) === normalizedSelector)) { if (selectors.length === 1) { return selectorBlock; } @@ -166,6 +164,53 @@ function extractClassBlock(cssText: string, selector: string): string { return ''; } +function splitCssSelectors(selectorsBlock: string): string[] { + const selectors: string[] = []; + let current = ''; + let parenDepth = 0; + + for (const char of selectorsBlock) { + if (char === '(') { + parenDepth += 1; + current += char; + continue; + } + + if (char === ')') { + parenDepth = Math.max(0, parenDepth - 1); + current += char; + continue; + } + + if (char === ',' && parenDepth === 0) { + const trimmed = current.trim(); + if (trimmed.length > 0) { + selectors.push(trimmed); + } + current = ''; + continue; + } + + current += char; + } + + const trimmed = current.trim(); + if (trimmed.length > 0) { + selectors.push(trimmed); + } + + return selectors; +} + +function normalizeCssSelector(selector: string): string { + return selector + .replace(/\s+/g, ' ') + .replace(/\(\s+/g, '(') + .replace(/\s+\)/g, ')') + .replace(/\s*,\s*/g, ', ') + .trim(); +} + test('computeWordClass preserves known and n+1 classes while adding JLPT classes', () => { const knownJlpt = createToken({ isKnown: true, @@ -669,9 +714,21 @@ test('JLPT CSS rules use underline-only styling in renderer stylesheet', () => { ); assert.match(jlptTooltipKeyboardSelectedBlock, /opacity:\s*1;/); - assert.match( + const plainWordHoverBlock = extractClassBlock( cssText, - /#subtitleRoot\s+\.word:not\(\.word-known\):not\(\.word-n-plus-one\):not\(\.word-name-match\):not\(\.word-frequency-single\):not\(\s*\.word-frequency-band-1\s*\):not\(\.word-frequency-band-2\):not\(\.word-frequency-band-3\):not\(\.word-frequency-band-4\):not\(\s*\.word-frequency-band-5\s*\):hover\s*\{[\s\S]*?background:\s*var\(--subtitle-hover-token-background-color,\s*rgba\(54,\s*58,\s*79,\s*0\.84\)\);[\s\S]*?color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;[\s\S]*?-webkit-text-fill-color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;/, + '#subtitleRoot .word:not(.word-known):not(.word-n-plus-one):not(.word-name-match):not(.word-frequency-single):not(.word-frequency-band-1):not(.word-frequency-band-2):not(.word-frequency-band-3):not(.word-frequency-band-4):not(.word-frequency-band-5):hover', + ); + assert.match( + plainWordHoverBlock, + /background:\s*var\(--subtitle-hover-token-background-color,\s*rgba\(54,\s*58,\s*79,\s*0\.84\)\);/, + ); + assert.match( + plainWordHoverBlock, + /color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;/, + ); + assert.match( + plainWordHoverBlock, + /-webkit-text-fill-color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;/, ); const coloredWordHoverBlock = extractClassBlock(cssText, '#subtitleRoot .word.word-known:hover'); @@ -707,13 +764,31 @@ test('JLPT CSS rules use underline-only styling in renderer stylesheet', () => { assert.match(coloredCharHoverBlock, /background:\s*transparent;/); assert.match(coloredCharHoverBlock, /color:\s*inherit\s*!important;/); - assert.match( + const jlptOnlyHoverBlock = extractClassBlock( cssText, - /\.word:is\(\.word-jlpt-n1,\s*\.word-jlpt-n2,\s*\.word-jlpt-n3,\s*\.word-jlpt-n4,\s*\.word-jlpt-n5\):not\(\s*\.word-known\s*\):not\(\.word-n-plus-one\):not\(\.word-name-match\):not\(\.word-frequency-single\):not\(\.word-frequency-band-1\):not\(\s*\.word-frequency-band-2\s*\):not\(\.word-frequency-band-3\):not\(\.word-frequency-band-4\):not\(\.word-frequency-band-5\):hover\s*\{[\s\S]*?color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;[\s\S]*?-webkit-text-fill-color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;/, + '#subtitleRoot .word:is(.word-jlpt-n1, .word-jlpt-n2, .word-jlpt-n3, .word-jlpt-n4, .word-jlpt-n5):not(.word-known):not(.word-n-plus-one):not(.word-name-match):not(.word-frequency-single):not(.word-frequency-band-1):not(.word-frequency-band-2):not(.word-frequency-band-3):not(.word-frequency-band-4):not(.word-frequency-band-5):hover', ); assert.match( - cssText, - /\.word:is\(\.word-jlpt-n1,\s*\.word-jlpt-n2,\s*\.word-jlpt-n3,\s*\.word-jlpt-n4,\s*\.word-jlpt-n5\):not\(\s*\.word-known\s*\):not\(\.word-n-plus-one\):not\(\.word-name-match\):not\(\.word-frequency-single\):not\(\.word-frequency-band-1\):not\(\s*\.word-frequency-band-2\s*\):not\(\.word-frequency-band-3\):not\(\.word-frequency-band-4\):not\(\.word-frequency-band-5\)::selection[\s\S]*?color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;[\s\S]*?-webkit-text-fill-color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;/, + jlptOnlyHoverBlock, + /color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;/, + ); + assert.match( + jlptOnlyHoverBlock, + /-webkit-text-fill-color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;/, + ); + assert.match( + extractClassBlock( + cssText, + '#subtitleRoot .word:is(.word-jlpt-n1, .word-jlpt-n2, .word-jlpt-n3, .word-jlpt-n4, .word-jlpt-n5):not(.word-known):not(.word-n-plus-one):not(.word-name-match):not(.word-frequency-single):not(.word-frequency-band-1):not(.word-frequency-band-2):not(.word-frequency-band-3):not(.word-frequency-band-4):not(.word-frequency-band-5)::selection', + ), + /color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;/, + ); + assert.match( + extractClassBlock( + cssText, + '#subtitleRoot .word:is(.word-jlpt-n1, .word-jlpt-n2, .word-jlpt-n3, .word-jlpt-n4, .word-jlpt-n5):not(.word-known):not(.word-n-plus-one):not(.word-name-match):not(.word-frequency-single):not(.word-frequency-band-1):not(.word-frequency-band-2):not(.word-frequency-band-3):not(.word-frequency-band-4):not(.word-frequency-band-5)::selection', + ), + /-webkit-text-fill-color:\s*var\(--subtitle-hover-token-color,\s*#f4dbd6\)\s*!important;/, ); const selectionBlock = extractClassBlock(cssText, '#subtitleRoot::selection');