Skip to content

Commit 632652d

Browse files
committed
refactor(ci): use cargo tree for conformance job detection (#14269)
## Summary - Replace `dorny/paths-filter` with `cargo tree`-based detection for more accurate dependency tracking in the conformance CI job - Extract common file detection and utility functions into reusable modules - Eliminate code duplication across CI scripts ## Changes ### New Files - **`.github/scripts/get-changed-files.js`** - Reusable module for detecting changed files from GitHub events (PR/push) - **`.github/scripts/check-conformance-changes.js`** - Conformance detection using `cargo tree -p oxc_coverage -p oxc_transform_conformance -p oxc_prettier_conformance` - **`.github/scripts/utils.js`** - Shared utilities: - `exec()` - Shell command execution with error handling - `getCrateDependencies()` - Generic cargo tree wrapper - `checkFilesAffectCrates()` - File-to-crate impact checking ### Modified Files - **`.github/scripts/generate-benchmark-matrix.js`** - Refactored to use shared modules - **`.github/workflows/ci.yml`** - Conformance job now uses `check-conformance-changes.js` instead of `dorny/paths-filter` ## Benefits - **More accurate**: Uses actual crate dependencies from `cargo tree` instead of static path patterns - **Maintainable**: Shared utilities eliminate code duplication - **Extensible**: Easy to add new scripts that need similar functionality - **Consistent**: Same file detection logic across all CI scripts ## Test Plan - [x] Verify CI workflow syntax is valid - [ ] Monitor first PR run to ensure conformance detection works correctly - [ ] Check that conformance is skipped when appropriate (e.g., linter-only changes) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 parent 653aa8a commit 632652d

File tree

5 files changed

+334
-123
lines changed

5 files changed

+334
-123
lines changed
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
#!/usr/bin/env node
2+
3+
/**
4+
* Check if conformance tests should run based on changed files.
5+
* Uses cargo tree to determine dependencies of oxc_coverage crate.
6+
*/
7+
8+
const { getChangedFiles } = require('./get-changed-files.js');
9+
const { getCrateDependencies } = require('./utils.js');
10+
11+
/**
12+
* Get all crates that conformance tests depend on
13+
* @returns {string[]} Array of crate names
14+
*/
15+
function getCoverageDependencies() {
16+
const packages = ['oxc_coverage', 'oxc_transform_conformance', 'oxc_prettier_conformance'];
17+
const deps = getCrateDependencies(packages);
18+
19+
console.error(`Conformance dependencies (${deps.length}):`);
20+
console.error(` ${deps.join(', ')}`);
21+
22+
return deps;
23+
}
24+
25+
/**
26+
* Directories that should always trigger conformance tests
27+
*/
28+
const ALWAYS_RUN_PATHS = [
29+
'tasks/coverage/',
30+
'tasks/common/',
31+
];
32+
33+
/**
34+
* Check if conformance tests should run based on changed files
35+
* @param {string[] | null} changedFiles - Array of changed file paths, or null for "run all"
36+
* @returns {boolean} True if conformance should run
37+
*/
38+
function shouldRunConformance(changedFiles) {
39+
// null means manual trigger or error - always run
40+
if (changedFiles === null) {
41+
console.error('No changed files list (manual trigger or error) - will run conformance');
42+
return true;
43+
}
44+
45+
// No files changed - skip conformance
46+
if (changedFiles.length === 0) {
47+
console.error('No files changed - will skip conformance');
48+
return false;
49+
}
50+
51+
// Check for paths that should always trigger conformance
52+
for (const file of changedFiles) {
53+
for (const path of ALWAYS_RUN_PATHS) {
54+
if (file.startsWith(path)) {
55+
console.error(`File ${file} matches always-run path ${path} - will run conformance`);
56+
return true;
57+
}
58+
}
59+
}
60+
61+
// Get dependencies
62+
const dependencies = getCoverageDependencies();
63+
64+
if (dependencies.length === 0) {
65+
console.error('Warning: No dependencies found - will run conformance as fallback');
66+
return true;
67+
}
68+
69+
// Check if any changed file affects a dependency crate
70+
for (const dep of dependencies) {
71+
const cratePath = `crates/${dep}/`;
72+
for (const file of changedFiles) {
73+
if (file.startsWith(cratePath)) {
74+
console.error(`File ${file} affects dependency ${dep} - will run conformance`);
75+
return true;
76+
}
77+
}
78+
}
79+
80+
console.error('No files affect oxc_coverage dependencies - will skip conformance');
81+
return false;
82+
}
83+
84+
/**
85+
* Main entry point
86+
*/
87+
async function main() {
88+
try {
89+
const changedFiles = await getChangedFiles();
90+
const shouldRun = shouldRunConformance(changedFiles);
91+
92+
// Output for GitHub Actions
93+
console.log(shouldRun ? 'true' : 'false');
94+
95+
// Set GitHub Actions notice
96+
if (shouldRun) {
97+
console.error('::notice title=Conformance tests::Will run conformance tests');
98+
} else {
99+
console.error('::notice title=Conformance tests::Will skip conformance tests');
100+
}
101+
102+
process.exit(0);
103+
} catch (error) {
104+
console.error('Error checking conformance changes:', error);
105+
// On error, run conformance as a fallback
106+
console.log('true');
107+
console.error('::warning title=Conformance check error::Error occurred, running conformance as fallback');
108+
process.exit(0);
109+
}
110+
}
111+
112+
// Run the script
113+
void main();

.github/scripts/generate-benchmark-matrix.js

Lines changed: 8 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
* This script determines which benchmark components need to run based on changed files.
66
*/
77

8-
const { execSync } = require('child_process');
98
const process = require('process');
10-
const https = require('https');
9+
const { getChangedFiles } = require('./get-changed-files.js');
10+
const { getCrateDependencies } = require('./utils.js');
1111

1212
// All available benchmark components
1313
const ALL_COMPONENTS = ['lexer', 'parser', 'transformer', 'semantic', 'minifier', 'codegen', 'formatter', 'linter'];
@@ -20,108 +20,6 @@ const GLOBAL_FILES = [
2020
'.github/scripts/generate-benchmark-matrix.js',
2121
];
2222

23-
/**
24-
* Execute a shell command and return the output
25-
* @param {string} command - Command to execute
26-
* @returns {string} Command output
27-
*/
28-
function exec(command) {
29-
try {
30-
return execSync(command, { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'] }).trim();
31-
} catch (error) {
32-
console.error(error);
33-
return '';
34-
}
35-
}
36-
37-
/**
38-
* Make a GitHub API request
39-
* @param {string} path - API path
40-
* @returns {Promise<any>} API response
41-
*/
42-
function githubApi(path) {
43-
return new Promise((resolve, reject) => {
44-
const options = {
45-
hostname: 'api.github.com',
46-
path,
47-
headers: {
48-
'User-Agent': 'oxc-benchmark-matrix',
49-
'Accept': 'application/vnd.github.v3+json',
50-
},
51-
};
52-
53-
// Add authorization if token is available
54-
const token = process.env.GITHUB_TOKEN;
55-
if (token) {
56-
options.headers['Authorization'] = `token ${token}`;
57-
}
58-
59-
https.get(options, (res) => {
60-
let data = '';
61-
res.on('data', chunk => data += chunk);
62-
res.on('end', () => {
63-
if (res.statusCode === 200) {
64-
resolve(JSON.parse(data));
65-
} else {
66-
reject(new Error(`GitHub API error: ${res.statusCode} ${data}`));
67-
}
68-
});
69-
}).on('error', reject);
70-
});
71-
}
72-
73-
/**
74-
* Get changed files based on the GitHub event type
75-
* @returns {Promise<string[]>} Array of changed file paths
76-
*/
77-
async function getChangedFiles() {
78-
const eventName = process.env.GITHUB_EVENT_NAME;
79-
const repository = process.env.GITHUB_REPOSITORY;
80-
const sha = process.env.GITHUB_SHA;
81-
const prNumber = process.env.GITHUB_PR_NUMBER;
82-
const ref = process.env.GITHUB_REF;
83-
84-
console.error(`Event: ${eventName}`);
85-
console.error(`Repository: ${repository}`);
86-
console.error(`SHA: ${sha}`);
87-
console.error(`Ref: ${ref}`);
88-
89-
if (eventName === 'workflow_dispatch') {
90-
console.error('Manual trigger - will run all benchmarks');
91-
return null; // Signal to run all benchmarks
92-
}
93-
94-
let files = [];
95-
96-
try {
97-
if (eventName === 'pull_request' && prNumber) {
98-
// For PR, use GitHub API to get changed files
99-
console.error(`Getting changed files for PR #${prNumber}`);
100-
const prFiles = await githubApi(`/repos/${repository}/pulls/${prNumber}/files?per_page=100`);
101-
files = prFiles.map(f => f.filename);
102-
} else if (sha && repository) {
103-
// For push to main, get the commit and compare with parent
104-
console.error(`Getting changed files for commit ${sha}`);
105-
const commit = await githubApi(`/repos/${repository}/commits/${sha}`);
106-
files = commit.files ? commit.files.map(f => f.filename) : [];
107-
} else {
108-
// No valid parameters for API calls
109-
console.error('Error: Missing required environment variables for GitHub API');
110-
console.error('Will run all benchmarks as fallback');
111-
return null; // Signal to run all benchmarks
112-
}
113-
} catch (error) {
114-
console.error(`Error getting changed files via API: ${error.message}`);
115-
console.error('Will run all benchmarks as fallback');
116-
return null; // Signal to run all benchmarks
117-
}
118-
119-
console.error(`Changed files (${files.length}):`);
120-
files.forEach(f => console.error(` - ${f}`));
121-
122-
return files;
123-
}
124-
12523
/**
12624
* Check if any global files have changed that affect all benchmarks
12725
* @param {string[]} changedFiles - Array of changed file paths
@@ -161,16 +59,16 @@ function getFeatureForComponent(component) {
16159
*/
16260
function getComponentDependencies(component) {
16361
const feature = getFeatureForComponent(component);
164-
const command =
165-
`cargo tree -p oxc_benchmark --features ${feature} --no-default-features -f "{lib}" -e normal --no-dedupe --prefix none 2>/dev/null | grep oxc | sort -u`;
166-
const output = exec(command);
62+
const deps = getCrateDependencies('oxc_benchmark', {
63+
features: feature,
64+
noDefaultFeatures: true,
65+
});
16766

168-
if (!output) {
67+
if (deps.length === 0) {
16968
console.error(`Warning: Could not get dependencies for ${component} (feature: ${feature})`);
170-
return [];
17169
}
17270

173-
return output.split('\n').filter(dep => dep && dep !== 'oxc_benchmark');
71+
return deps;
17472
}
17573

17674
/**
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
#!/usr/bin/env node
2+
3+
/**
4+
* Get changed files from GitHub events (pull request or push).
5+
* This module provides a reusable function for detecting changed files.
6+
*/
7+
8+
const https = require('https');
9+
const process = require('process');
10+
11+
/**
12+
* Make a GitHub API request
13+
* @param {string} path - API path
14+
* @returns {Promise<any>} API response
15+
*/
16+
function githubApi(path) {
17+
return new Promise((resolve, reject) => {
18+
const options = {
19+
hostname: 'api.github.com',
20+
path,
21+
headers: {
22+
'User-Agent': 'oxc-changed-files',
23+
'Accept': 'application/vnd.github.v3+json',
24+
},
25+
};
26+
27+
// Add authorization if token is available
28+
const token = process.env.GITHUB_TOKEN;
29+
if (token) {
30+
options.headers['Authorization'] = `token ${token}`;
31+
}
32+
33+
https.get(options, (res) => {
34+
let data = '';
35+
res.on('data', chunk => data += chunk);
36+
res.on('end', () => {
37+
if (res.statusCode === 200) {
38+
resolve(JSON.parse(data));
39+
} else {
40+
reject(new Error(`GitHub API error: ${res.statusCode} ${data}`));
41+
}
42+
});
43+
}).on('error', reject);
44+
});
45+
}
46+
47+
/**
48+
* Get changed files based on the GitHub event type
49+
* @returns {Promise<string[] | null>} Array of changed file paths, or null to signal "run all"
50+
*/
51+
async function getChangedFiles() {
52+
const eventName = process.env.GITHUB_EVENT_NAME;
53+
const repository = process.env.GITHUB_REPOSITORY;
54+
const sha = process.env.GITHUB_SHA;
55+
const prNumber = process.env.GITHUB_PR_NUMBER;
56+
const ref = process.env.GITHUB_REF;
57+
58+
console.error(`Event: ${eventName}`);
59+
console.error(`Repository: ${repository}`);
60+
console.error(`SHA: ${sha}`);
61+
console.error(`Ref: ${ref}`);
62+
63+
if (eventName === 'workflow_dispatch') {
64+
console.error('Manual trigger - returning null (run all)');
65+
return null; // Signal to run all
66+
}
67+
68+
let files = [];
69+
70+
try {
71+
if (eventName === 'pull_request' && prNumber) {
72+
// For PR, use GitHub API to get changed files
73+
console.error(`Getting changed files for PR #${prNumber}`);
74+
const prFiles = await githubApi(`/repos/${repository}/pulls/${prNumber}/files?per_page=100`);
75+
files = prFiles.map(f => f.filename);
76+
} else if (sha && repository) {
77+
// For push to main, get the commit and compare with parent
78+
console.error(`Getting changed files for commit ${sha}`);
79+
const commit = await githubApi(`/repos/${repository}/commits/${sha}`);
80+
files = commit.files ? commit.files.map(f => f.filename) : [];
81+
} else {
82+
// No valid parameters for API calls
83+
console.error('Error: Missing required environment variables for GitHub API');
84+
console.error('Returning null (run all) as fallback');
85+
return null; // Signal to run all
86+
}
87+
} catch (error) {
88+
console.error(`Error getting changed files via API: ${error.message}`);
89+
console.error('Returning null (run all) as fallback');
90+
return null; // Signal to run all
91+
}
92+
93+
console.error(`Changed files (${files.length}):`);
94+
files.forEach(f => console.error(` - ${f}`));
95+
96+
return files;
97+
}
98+
99+
module.exports = { getChangedFiles };
100+
101+
// If run directly as a script, output changed files as JSON
102+
if (require.main === module) {
103+
getChangedFiles()
104+
.then(files => {
105+
console.log(JSON.stringify(files));
106+
process.exit(0);
107+
})
108+
.catch(error => {
109+
console.error('Error:', error);
110+
console.log(JSON.stringify(null));
111+
process.exit(1);
112+
});
113+
}

0 commit comments

Comments
 (0)