-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: adding further fixtures #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive test fixtures for advanced CSS if() function features, expanding test coverage for complex conditions and runtime-only scenarios. The changes focus on testing edge cases and advanced functionality that cannot be transformed at build time.
Key changes:
- Added 6 new test fixtures covering complex supports conditions, nested media queries, boolean negation, cyclic substitution, empty token streams, and multiple mixed conditions
- Introduced a new
runtimeOnlyFixtureTests
category for features that require runtime processing - Updated test suites to handle both build-time transformable and runtime-only fixtures
- Added a utility script for generating expected fixture outputs
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
test/scripts/fixture-utils.js | Adds new fixture test configurations and runtime-only test category |
test/fixtures/*.css | New input/expected CSS fixture pairs for advanced if() function scenarios |
packages/postcss-if-function/test/plugin.test.js | Updates PostCSS plugin tests to handle runtime-only fixtures |
packages/postcss-if-function/src/index.js | Improves build-time transformation logic for runtime-only features |
packages/css-if-polyfill/test/integrated.test.js | Adds integrated tests for runtime-only fixture scenarios |
generate-fixtures.js | Utility script for generating expected outputs from input fixtures |
.github/copilot-instructions.md | Adds TODO comment referencing specification updates |
Comments suppressed due to low confidence (1)
test/fixtures/multiple-mixed-conditions.expected.css:17
- The expected output doesn't include transformation for the
style(--large-padding)
condition from the input. According to the WCAG specification, style queries should be evaluated and transformed into appropriate CSS rules, but this condition appears to be missing from the expected output.
}
.test { | ||
display: grid; | ||
} | ||
@supports (not display: grid) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot generated this review using guidance from copilot-instructions.md.
generate-fixtures.js
Outdated
'empty-token-stream', | ||
'cyclic-substitution', | ||
'complex-supports', | ||
'boolean-negation', | ||
'multiple-mixed-conditions', | ||
'nested-media-features' | ||
]; | ||
|
||
async function generateExpectedOutputs() { | ||
const results = await Promise.all( | ||
newFixtures.map(async (fixture) => { | ||
const inputPath = path.join(fixturesDir, `${fixture}.input.css`); | ||
const expectedPath = path.join( | ||
fixturesDir, | ||
`${fixture}.expected.css` | ||
); | ||
|
||
try { | ||
const input = await readFile(inputPath, 'utf8'); | ||
console.log(`Processing ${fixture}...`); | ||
console.log('Input:', input); | ||
|
||
const result = buildTimeTransform(input); | ||
console.log('Output:', result); | ||
|
||
// Extract the appropriate CSS output (nativeCSS for static transforms, runtimeCSS for dynamic) | ||
const outputCSS = result.nativeCSS || result.runtimeCSS || ''; | ||
|
||
await writeFile(expectedPath, outputCSS); | ||
console.log(`✓ Generated ${fixture}.expected.css\n`); | ||
return { fixture, success: true }; | ||
} catch (error) { | ||
console.error(`✗ Failed to process ${fixture}:`, error.message); | ||
return { fixture, success: false, error }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback logic for selecting output CSS may produce inconsistent results. For runtime-only fixtures, you should explicitly choose runtimeCSS
, and for build-time fixtures, choose nativeCSS
. The current fallback could mask issues where the wrong transformation type is being used.
'empty-token-stream', | |
'cyclic-substitution', | |
'complex-supports', | |
'boolean-negation', | |
'multiple-mixed-conditions', | |
'nested-media-features' | |
]; | |
async function generateExpectedOutputs() { | |
const results = await Promise.all( | |
newFixtures.map(async (fixture) => { | |
const inputPath = path.join(fixturesDir, `${fixture}.input.css`); | |
const expectedPath = path.join( | |
fixturesDir, | |
`${fixture}.expected.css` | |
); | |
try { | |
const input = await readFile(inputPath, 'utf8'); | |
console.log(`Processing ${fixture}...`); | |
console.log('Input:', input); | |
const result = buildTimeTransform(input); | |
console.log('Output:', result); | |
// Extract the appropriate CSS output (nativeCSS for static transforms, runtimeCSS for dynamic) | |
const outputCSS = result.nativeCSS || result.runtimeCSS || ''; | |
await writeFile(expectedPath, outputCSS); | |
console.log(`✓ Generated ${fixture}.expected.css\n`); | |
return { fixture, success: true }; | |
} catch (error) { | |
console.error(`✗ Failed to process ${fixture}:`, error.message); | |
return { fixture, success: false, error }; | |
{ name: 'empty-token-stream', type: 'build-time' }, | |
{ name: 'cyclic-substitution', type: 'build-time' }, | |
{ name: 'complex-supports', type: 'build-time' }, | |
{ name: 'boolean-negation', type: 'runtime-only' }, | |
{ name: 'multiple-mixed-conditions', type: 'runtime-only' }, | |
{ name: 'nested-media-features', type: 'build-time' } | |
]; | |
async function generateExpectedOutputs() { | |
const results = await Promise.all( | |
newFixtures.map(async ({ name, type }) => { | |
const inputPath = path.join(fixturesDir, `${name}.input.css`); | |
const expectedPath = path.join( | |
fixturesDir, | |
`${name}.expected.css` | |
); | |
try { | |
const input = await readFile(inputPath, 'utf8'); | |
console.log(`Processing ${name}...`); | |
console.log('Input:', input); | |
const result = buildTimeTransform(input); | |
console.log('Output:', result); | |
// Select the appropriate CSS output based on fixture type | |
const outputCSS = | |
type === 'build-time' ? result.nativeCSS : | |
type === 'runtime-only' ? result.runtimeCSS : | |
''; | |
if (!outputCSS) { | |
throw new Error(`No valid CSS output for fixture type: ${type}`); | |
} | |
await writeFile(expectedPath, outputCSS); | |
console.log(`✓ Generated ${name}.expected.css\n`); | |
return { fixture: name, success: true }; | |
} catch (error) { | |
console.error(`✗ Failed to process ${name}:`, error.message); | |
return { fixture: name, success: false, error }; |
Copilot uses AI. Check for mistakes.
Description
Adding new test fixtures.
Type of Change
Testing
Checklist
Browser Testing
If applicable, please test in the following browsers:
Performance Impact
If this change affects performance:
Breaking Changes
If this introduces breaking changes, please describe:
Additional Notes
Any additional information that reviewers should know.