Extract JavaScript backend test resources to files#23066
Extract JavaScript backend test resources to files#23066riisi wants to merge 4 commits intopantsbuild:mainfrom
Conversation
Migrate test_integration_test.py to use external test resource files instead of inlined rule_runner.write_files() calls. This follows the pattern established in the TypeScript backend and makes lockfiles easier to maintain with actual package manager commands. Changes: - Add test_resources/ directory with jest_project and mocha_project - Add testutil.py with load_js_test_project() helper - Refactor tests to load base projects and override specific files - Remove unused fixtures (_find_lockfile_resource, jest_lockfile, etc.) Ref: pantsbuild#22807 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
benjyw
left a comment
There was a problem hiding this comment.
Looks good to me, but you can tell from my question that I am not very familiar with this code.
| "test_integration_test.py": { | ||
| "dependencies": ["./jest_resources", "./mocha_resources"], | ||
| "dependencies": [ | ||
| "./jest_resources", |
There was a problem hiding this comment.
For my education: what is the distinction between these jest_resources and the ones under test_resources/jest_project?
|
Just need to hold back on the merge here. There were a couple of things I saw briefly I didn't understand, but need to go back through the code properly and check out. @riisi Is there any LLM disclosure here, for places for us to focus on reviewing? Most of this looked good - but there were a couple of changes I didn't understand at a glance when I looked at this last week. Need to carve out some time this week to review properly - it's mostly a mechanical change, but it's a big one. |
- Move test_resources/ from javascript/ into goals/, colocating resources with their only consumer (test_integration_test.py) - Move testutil.py into goals/ for the same reason - Consolidate jest_resources/ and mocha_resources/ into test_resources/jest_project/ and test_resources/mocha_project/, eliminating confusing overlap between old and new resource dirs - Delete unused basic_package/ (no test references it) - Replace broad resources() glob with granular per-project targets: :jest_resources and :mocha_resources, so each test declares only the resources it actually needs - Read jest_dev_dependencies fixture from package.json instead of hardcoding versions, making package.json the single source of truth - Update export_test.py to read lockfile from consolidated location - Clean up stale BUILD deps and README references Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
I've removed the stale reference that @benjyw picked up. I've also tried to make the naming and location clearer / closer to consumers, and deps more granular. I think I was trying to minimise changes in the initial pass, but I think the clarity is better. @sureshjoshi I'm using Claude here - I tried to make that implicit via commit message, but haven't been following the disclosure discussions sorry. |
Migrate test_integration_test.py to use external test resource files instead of inlined rule_runner.write_files() calls. This follows the pattern established in the TypeScript backend and makes lockfiles easier to maintain with actual package manager commands.
Changes:
Ref: #22807
Summary of changes (Claude):