-
Notifications
You must be signed in to change notification settings - Fork 0
Add GitHub Action with file-based caching, remove Redis dependency #2
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
Open
cosmith
wants to merge
30
commits into
main
Choose a base branch
from
claude/remove-redis-dependency-aiYyi
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replace external Redis database with a simple local JSON file storage: - Add src/backend/fileStorage.ts with same saveTimings/getTimings interface - Remove src/backend/redis.ts and redis package dependency - Update config.ts to use FAIRSPLICE_TIMINGS_FILE env var (default: .fairsplice-timings.json) - Update commands to use new file-based storage - Update README with new configuration and GitHub Actions cache example This eliminates the need for any external database while maintaining the same functionality (storing last 10 timings per test file).
- Add GitHub Actions workflow (test.yml) with three jobs: - unit-tests: runs existing unit tests in src/ - dummy-tests: runs dummy tests, saves timings, and splits for workers - fairsplice-integration: matrix job testing split across 3 workers - Add dummy test files with varying execution times: - fast.test.ts: ~100ms total (50ms, 30ms, 20ms delays) - medium.test.ts: ~450ms total (150ms, 200ms, 100ms delays) - slow.test.ts: ~900ms total (300ms, 350ms, 250ms delays) - variable.test.ts: ~600ms total (25ms, 175ms, 400ms delays) - Add fileStorage.test.ts to test the JSON file storage backend
Add ASCII diagram explaining the two-phase workflow: 1. Split phase: load timings, distribute tests via bin packing 2. Save phase: extract timings from JUnit XML, update storage Includes key concepts section explaining bin packing and rolling averages.
The integration test was incorrectly running all tests in each worker before splitting. Now it properly simulates a real CI workflow: 1. integration-prepare: Restores cached timings (or generates once on first run), splits tests, uploads split config as artifact 2. integration-worker (x3): Downloads split config, runs ONLY assigned tests, uploads JUnit results 3. integration-finalize: Collects all worker results, saves updated timings to cache for next run This demonstrates how fairsplice is intended to be used: - Timings persist across CI runs via cache - Tests run only once (not duplicated across workers) - Each worker runs its fair share based on historical timings
Remove the step that ran all tests to generate initial timings - this defeats the purpose of test splitting in a real scenario where the full suite could take hours. Now the workflow correctly handles cold starts: - First run: No cached timings, fairsplice uses default timing (10s) per test, tests are split evenly across workers - After run: Timings are saved from worker results - Subsequent runs: Cached timings enable optimal distribution This matches real-world usage where you can't afford to run the full test suite just to gather timing data.
Workers only need to: 1. Download split.json artifact 2. Parse JSON to get test list (using jq) 3. Run their test framework Changes: - Remove 'bun install' from workers (not needed) - Use jq for JSON parsing (cleaner than inline bun script) - Add comments clarifying workers don't need fairsplice - Use GitHub outputs for cleaner conditional flow
Replace implicit default file path with explicit --timings-file parameter: - Both save and split commands now require --timings-file - Remove TIMINGS_FILE_PATH from config and environment variable support - Update fileStorage functions to accept filePath as first argument - Update README with new usage examples
Replace implicit default file path with explicit --timings-file parameter: - Both save and split commands now require --timings-file - Remove TIMINGS_FILE_PATH from config and environment variable support - Update fileStorage functions to accept filePath as first argument - Update README with new usage examples and diagram
…iYyi' into claude/add-github-actions-tests-gplq6
New command: fairsplice merge --timings-file <file> --prefix <prefix> - Finds all files matching the prefix pattern (e.g., junit-*.xml) - Parses and aggregates timings from all matched JUnit XML files - Saves combined timings to the specified timings file This is useful in CI pipelines where tests run in parallel and each worker produces its own JUnit XML file.
The merge command with a prefix that matches a single file works identically to save, making save redundant. Simplify to two commands: - merge: Read JUnit XML file(s) → save timings - split: Read timings → output test distribution
New action.yml provides a simple interface for GitHub Actions:
- Split: distributes tests across workers with index output
- Merge: saves timings from JUnit XML files
- Automatic cache restore/save - no manual setup needed
Usage:
uses: dashdoc/fairsplice@v1
with:
command: split
pattern: 'tests/**/*.py'
total: 3
index: ${{ matrix.index }}
Simplified workflow that tests the action itself: - Unit tests for src/ - Integration tests using the action with 3 parallel workers - Save timings job that merges results
- Use bun run ${{ github.action_path }}/index.ts instead of bunx fairsplice@latest
- Add bun install step to install dependencies
- This allows testing the action from the current repo with uses: ./
Separate concerns by introducing a new convert command: - convert: JUnit XML → timing JSON (single file) - merge: timing JSONs → combined timing history - split: timing history + pattern → test distribution This allows each worker to convert their JUnit results to timing JSON, then merge job combines all timing JSONs together.
- Add cache-key input to differentiate frontend/backend workflows - Remove branch reference from cache key to share timings across branches - Cache is now shared across all branches by default
- Make cache-key input required to prevent configuration mistakes - Fix cache key format: use run_id suffix for saves, prefix matching for restores - Add cache-key to test workflow - Document cache behavior in README (branch scoping, cross-branch sharing) - Add CLAUDE.md for Claude Code guidance Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Make cache-key not required at schema level (only needed for split/merge) - Add validation step to ensure cache-key is provided for split/merge - Update README example to show correct flow with convert step - Document convert command inputs in reference table - Fix merge description (expects timing JSON, not JUnit XML) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When all tests fail or are skipped, there won't be any timing artifacts. The merge command should handle this gracefully instead of failing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When tests are skipped or fail early, the JUnit XML file won't exist. The convert command should handle this gracefully instead of failing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Playwright and other test runners may output paths relative to their config directory, but split patterns use repo-root paths. The path-prefix option allows prepending a prefix to all file paths during conversion so they match the split pattern. Example: --path-prefix "frontends/apps/e2e/" converts "tests/foo.spec.ts" to "frontends/apps/e2e/tests/foo.spec.ts" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
GitHub cache keys are immutable, so re-running a workflow would fail to save because the key already exists. Adding run_attempt ensures each attempt gets a unique key. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update Quick Start to show the recommended pattern of computing splits in a dedicated job and passing to test jobs via outputs. This ensures that re-running a failed job runs the same tests, since GitHub Actions preserves workflow outputs on re-runs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update Quick Start to show the recommended pattern of computing splits in a dedicated job and passing to test jobs via outputs. This ensures that re-running a failed job runs the same tests, since GitHub Actions preserves workflow outputs on re-runs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
convertcommand to transform JUnit XML to timing JSONmergecommand to aggregate timing files from parallel workersTest plan