From 577094cf1134b9e6f6907ec6b1b00841067f8d39 Mon Sep 17 00:00:00 2001 From: Matt Godbolt Date: Sun, 22 Jun 2025 14:34:16 -0500 Subject: [PATCH] Add vitest related to pre-commit with expensive test skipping (#7854) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This PR improves the pre-commit hook performance by: - Using `vitest related` to run only tests affected by changed files - Adding ability to skip expensive tests (filter tests) during pre-commit - Providing a consistent mechanism for skipping expensive tests ## Changes - Modified `lint-staged.config.mjs` to run `vitest related` with `SKIP_EXPENSIVE_TESTS=true` - Updated `test/filter-tests.ts` to use idiomatic `describe.skipIf()` for conditional test execution - Changed `test-min` script to use `SKIP_EXPENSIVE_TESTS` environment variable instead of `--exclude` - Updated `CLAUDE.md` with documentation about the new test workflow ## Impact - Pre-commit hooks are now much faster as they: - Only run tests related to changed files - Skip 688 expensive filter tests - Use the same skipping mechanism as `npm run test-min` ## Testing - ✅ Verified `vitest related` correctly identifies and runs related tests - ✅ Confirmed filter tests are skipped when `SKIP_EXPENSIVE_TESTS=true` - ✅ Tested that full test suite still runs all tests when env var is not set - ✅ Pre-commit hooks work correctly with the new setup 🤖 Generated with [Claude Code](https://claude.ai/code) --------- Co-authored-by: Claude --- CLAUDE.md | 7 +++++++ CONTRIBUTING.md | 3 ++- lint-staged.config.mjs | 2 +- package.json | 2 +- test/filter-tests.ts | 4 ++-- test/utils.ts | 3 +++ 6 files changed, 16 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 225207486..f40cf7aa7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,6 +84,13 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co - Write platform-specific assertions - Use path-agnostic checks +### Test Execution with Expensive Test Skipping +- The `SKIP_EXPENSIVE_TESTS=true` environment variable skips expensive tests (like filter tests) +- Pre-commit hooks use `vitest related` to run only tests related to changed files +- Use `npm run test-min` to run tests with expensive tests skipped +- Use `npm run test` to run all tests including expensive ones +- To mark tests as expensive, use: `describe.skipIf(process.env.SKIP_EXPENSIVE_TESTS === 'true')('Test suite', () => {...})` + ## Compiler Testing Specifics - Mock filesystem operations when testing file I/O - Use `makeFakeCompilerInfo()` for creating test compiler configurations diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 288912950..66fac6a76 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,7 +36,8 @@ when testing your changes locally. warnings. - If you're adding a new server-side component, please do your best to add a test to cover it. For client-side changes that's trickier. -- Tests should run automatically as a pre-commit step. _You can disable this check with `git commit --no-verify` if +- Tests should run automatically as a pre-commit step. The pre-commit hook runs only tests related to changed files + and skips expensive tests (like filter tests) for faster feedback. _You can disable this check with `git commit --no-verify` if needed_. - You can run `make check` to run both the linter and the code tests - Do a smoke test: Run `make` and ensure the site works as you'd expect. Concentrate on the areas you'd expect to have diff --git a/lint-staged.config.mjs b/lint-staged.config.mjs index 643ce17c7..e9633b222 100644 --- a/lint-staged.config.mjs +++ b/lint-staged.config.mjs @@ -1,4 +1,4 @@ export default { - '*.ts': ['npm run lint', () => 'npm run ts-check'], + '*.ts': ['npm run lint', () => 'npm run ts-check', 'cross-env SKIP_EXPENSIVE_TESTS=true npx vitest related --run'], '*.{html,md,js}': ['npm run lint'], }; diff --git a/package.json b/package.json index 825948801..32f39da96 100644 --- a/package.json +++ b/package.json @@ -147,7 +147,7 @@ "test-coverage": "vitest run --coverage", "test": "vitest run", "test:watch": "vitest", - "test-min": "vitest run --exclude test/filter-tests.ts", + "test-min": "cross-env SKIP_EXPENSIVE_TESTS=true vitest run", "check": "npm run ts-check && npm run lint-check && npm run check-frontend-imports && npm run test-min -- --reporter dot", "check-frontend-imports": "node ./etc/scripts/check-frontend-imports.js", "dev": "cross-env NODE_ENV=DEV node --no-warnings=ExperimentalWarning --import=tsx app.ts", diff --git a/test/filter-tests.ts b/test/filter-tests.ts index 06eda3d95..cb6be7301 100644 --- a/test/filter-tests.ts +++ b/test/filter-tests.ts @@ -29,7 +29,7 @@ import {describe, expect, it} from 'vitest'; import {ParseFiltersAndOutputOptions} from '../types/features/filters.interfaces.js'; -import {processAsm, resolvePathFromTestRoot} from './utils.js'; +import {processAsm, resolvePathFromTestRoot, skipExpensiveTests} from './utils.js'; const casesRoot = resolvePathFromTestRoot('filters-cases'); const files = fs.readdirSync(casesRoot); @@ -70,7 +70,7 @@ function testFilter(filename: string, suffix: string, filters: ParseFiltersAndOu }, 10000); // Bump the timeout a bit so that we don't fail for slow cases } -describe('Filter test cases', () => { +describe.skipIf(skipExpensiveTests)('Filter test cases', () => { if (process.platform === 'win32' || process.platform === 'darwin') { it('should skip filter-tests on Windows', () => { expect(true).toBe(true); diff --git a/test/utils.ts b/test/utils.ts index 336d70c88..0b674f191 100644 --- a/test/utils.ts +++ b/test/utils.ts @@ -29,6 +29,9 @@ import {fileURLToPath} from 'node:url'; import {afterEach, expect, onTestFinished} from 'vitest'; import * as temp from '../lib/temp.js'; +// Check if expensive tests should be skipped (e.g., during pre-commit hooks) +export const skipExpensiveTests = process.env.SKIP_EXPENSIVE_TESTS === 'true'; + import {CompilationEnvironment} from '../lib/compilation-env.js'; import {CompilationQueue} from '../lib/compilation-queue.js'; import {CC65AsmParser} from '../lib/parsers/asm-parser-cc65.js';