fix(js): add test coverage detection for JavaScript modules#29
fix(js): add test coverage detection for JavaScript modules#29samtuckerdavis merged 3 commits intomainfrom
Conversation
- Add test_coverage.py to JavaScript language plugin with naming convention mapping (strip_test_markers, map_test_to_source), import spec parsing for ESM and CommonJS require(), and quality detection patterns (ASSERT_PATTERNS, TEST_FUNCTION_RE) - Register test_coverage hooks via generic_lang test_coverage_module parameter in javascript/__init__.py - Fix path resolution bug in ts_build_dep_graph where resolve_import returning a project-root-relative path was incorrectly joined with the scan path (already containing the subdir prefix), causing all imports to resolve outside the file_set Without these fixes, JavaScript projects using .test.js naming convention showed 0% test health regardless of test coverage.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 42 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefines Treesitter import dependency resolution to accept project-root-relative resolver outputs without forced normalization. Adds a JavaScript test-coverage module with import extraction, test-to-source mapping, and import-spec resolution, and wires it into the JavaScript language plugin. Changes
Sequence Diagram(s)sequenceDiagram
participant TestFile as Test File
participant Coverage as JS Test Coverage Module
participant FS as Production Files (set)
participant Graph as Import Graph
Note over TestFile,Coverage: parse_test_import_specs(content)
TestFile->>Coverage: provide file content
Coverage->>Coverage: extract import specs (ESM/CJS/dynamic)
Coverage->>FS: resolve relative specs (resolve_import_spec)
FS-->>Coverage: matched production path or None
Coverage->>Coverage: map_test_to_source(test_path, production_set)
Coverage-->>TestFile: mapped production path / None
Note over Coverage,Graph: graph import edges updated elsewhere using resolved paths
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Auto-Merge Gate Check — BlockedPR labeled Failed Gates
How to unblock
Permanent block
Auto-Merge Gate Check — 2026-04-18T16:06:04.175Z |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@desloppify/languages/_framework/treesitter/imports/graph.py`:
- Around line 78-87: Add a characterization test that reproduces the bug where
resolve_import returns a project-root-relative path and the code incorrectly
joins it with scan_path: in the test, stub/mock resolve_import to return a
project-root-relative path that is present in file_set, set scan_path different
from project root so the pre-fix code would join it to produce a wrong absolute
path, then assert that the graph built by the module (the function that
populates graph and uses file_set, resolve_import, and scan_path) contains the
incorrect import path prior to applying the fix and that after the fix the
import recorded is the original project-root-relative path (i.e., no scan_path
join). Use identifiers file_set, resolve_import, scan_path, and the
graph-building function from imports/graph.py to locate and hook behavior.
In `@desloppify/languages/javascript/test_coverage.py`:
- Around line 50-57: Change parse_test_import_specs to return a set[str] instead
of list[str]: update the function signature and build/return a set of specs (use
a set comprehension or add specs into a set while iterating over
_IMPORT_RE.finditer(content)) so duplicates are removed and semantics match the
Python/TypeScript hooks; keep the extraction logic using m.group(1) or
m.group(2) but insert each non-empty spec into the set before returning it.
- Around line 79-87: The current match logic can return wrong files on basename
collisions because it iterates production_set first nondeterministically; change
the logic to first check for exact path matches by iterating candidates and
returning any candidate present in production_set (use the existing check `if c
in production_set: return c`), and only if no exact match is found, build a
deterministic basename fallback: map basenames (os.path.basename) from
production_set to a sorted list of full paths and then for each candidate check
its basename against that map and return the first production path from the
sorted list; update the code around variables production_set, candidates,
prod_base, and os.path.basename to implement this order and deterministic
selection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d1eb4c24-c6a9-4891-93ce-fd1364d71605
📒 Files selected for processing (3)
desloppify/languages/_framework/treesitter/imports/graph.pydesloppify/languages/javascript/__init__.pydesloppify/languages/javascript/test_coverage.py
…asename, characterization test - Change parse_test_import_specs return type from list[str] to set[str] to deduplicate specs and match Python/TypeScript hook semantics - Make map_test_to_source deterministic on basename collisions: exact path match first, then sorted basename fallback using a stable basename-to-path mapping - Add characterization test for the graph.py project-root-relative path resolution bug: verifies that resolve_import returning a relative path in file_set is recorded directly without being incorrectly joined with scan_path - Add tests/lang/javascript/ with full coverage of JavaScript test_coverage.py hooks
|
Addressed all three CodeRabbit review items in the latest commit:
Also added All 6643 tests pass locally. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@desloppify/tests/lang/javascript/test_javascript_test_coverage.py`:
- Around line 10-22: The test test_parse_test_import_specs_handles_esm_and_cjs
is missing an assertion for the side-effect import; update the test in
desloppify/tests/lang/javascript/test_javascript_test_coverage.py to assert that
js_cov.parse_test_import_specs(content) includes "./side-effect.js" (i.e., add
an assertion like assert "./side-effect.js" in specs) so that
parse_test_import_specs is validated for side-effect import extraction alongside
"./foo.js", "./bar", and "./lazy.js".
- Around line 70-77: The test
test_map_test_to_source_is_deterministic_on_basename_collision currently only
asserts stability but not the actual deterministic choice; update it to assert
the concrete expected path returned by js_cov.map_test_to_source for the input
production = {"a/util.js","b/util.js"} (i.e., assert result1 == "a/util.js") in
addition to the repeated-call stability check so the test fails if the intended
sorted-choice fallback changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a410f671-23d8-43a3-a3e1-84b66ad29531
📒 Files selected for processing (4)
desloppify/languages/javascript/test_coverage.pydesloppify/tests/lang/common/test_treesitter_imports_direct.pydesloppify/tests/lang/javascript/__init__.pydesloppify/tests/lang/javascript/test_javascript_test_coverage.py
samtuckerdavis
dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot]
All three requested changes addressed in latest commit: set return type for parse_test_import_specs, deterministic basename fallback in map_test_to_source, and characterization test for graph.py path resolution bug.
…ssertion and concrete deterministic value check - Assert './side-effect.js' is extracted by parse_test_import_specs (side-effect import coverage) - Assert map_test_to_source returns 'a/util.js' (lexicographically first) on basename collision, not just stability — pins the intended sorted-choice fallback behavior
1 participant
Summary
test_coverage.pyto the JavaScript language plugin with naming convention mapping, import spec parsing (ESM and CommonJS), and test quality patternsts_build_dep_graphwhere project-root-relative paths fromresolve_js_importwere double-joined with the scan path prefixgeneric_lang test_coverage_moduleparameterProblem
JavaScript projects using
.test.jsnaming convention showed 0% test health regardless of test coverage. Two root causes:test_coverage.py, sonaming_based_mapping(which strips.test.from filenames to find production files) returned empty forlang_name='javascript'ts_build_dep_graphhad a path mismatch:resolve_js_importreturns project-root-relative paths (e.g._ui/server.js) but the graph builder joined them with the already-prefixed scan path (/path/to/_ui/), producing invalid absolute paths (/path/to/_ui/_ui/server.js) that never matched the file setTest plan
.test.jsfiles — should show > 0% test healthnaming_based_mapping('javascript')mapsfoo.test.js→foo.jsimport ... from './module.js'relationshipsrequire('./module')is also parsed byparse_test_import_specsSummary by CodeRabbit
New Features
Improvements
Tests