mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-04-09 16:19:25 -07:00
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
This commit is contained in:
62
backlog/tasks/task-114 - Fix-failing-CI-checks-on-PR-15.md
Normal file
62
backlog/tasks/task-114 - Fix-failing-CI-checks-on-PR-15.md
Normal file
@@ -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
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
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.
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
<!-- AC:BEGIN -->
|
||||
- [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
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
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.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
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`.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
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).
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
@@ -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');
|
||||
|
||||
Reference in New Issue
Block a user