Skip to content

fix: resolve jest-runner from project's node_modules for Jest 30 compatibility#1497

Merged
mohammedahmed18 merged 11 commits intomainfrom
fix/jest30-pnpm-resolution
Feb 17, 2026
Merged

fix: resolve jest-runner from project's node_modules for Jest 30 compatibility#1497
mohammedahmed18 merged 11 commits intomainfrom
fix/jest30-pnpm-resolution

Conversation

@mohammedahmed18
Copy link
Contributor

Summary

  • Fixes Jest 30 loop-runner error: TypeError: runtime.enterTestCode is not a function
  • Root cause: jest-runner was being loaded from codeflash's node_modules (v29) instead of project's (v30)
  • Adds generic recursive search that works with npm, yarn, and pnpm (including non-hoisted deps)

Changes

  • loop-runner.js: Replace pnpm-specific resolution with recursive search for jest-runner/package.json anywhere in node_modules tree
  • capture.js: Disable internal looping when external loop-runner is used (Jest handles batching)
  • test_runner.py: Remove debug print statement
  • parse.py: Remove unnecessary warning about loop indices

Test plan

  • Tested with novu project (pnpm monorepo, Jest 30.0.5)
  • Verified 5 batches run correctly with proper loop indices
  • Exit code 0

🤖 Generated with Claude Code

mohammedahmed18 and others added 2 commits February 16, 2026 14:31
…atibility

The loop-runner was loading jest-runner from codeflash's node_modules (v29)
instead of the project's (v30), causing "runtime.enterTestCode is not a function"
errors. This fix:

- Adds recursive search to find jest-runner in any node_modules structure
- Works with npm, yarn, and pnpm (including non-hoisted deps)
- Prefers higher versions when multiple are found
- Removes internal looping in capturePerf when using external loop-runner
- Creates fresh TestRunner per batch to avoid Jest 30 state corruption

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
}
}

const totalTimeMs = Date.now() - startTime;
Copy link
Contributor

@claude claude bot Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still present: totalTimeMs (line 367) is computed but never used. hasFailure (line 330) is declared but never read - the return value batchResult.hasFailure from _runAllTestsOnce is also never consumed in runTests.

function isValidJestRunnerPath(jestRunnerPath) {
if (!fs.existsSync(jestRunnerPath)) {
return false;
function findJestRunnerRecursive(nodeModulesPath, maxDepth = 5) {
Copy link
Contributor

@claude claude bot Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still present: createRequire is imported at line 33 but never used after the refactoring removed the internalRequire approach.

mohammedahmed18 and others added 3 commits February 16, 2026 14:35
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Comment on lines 88 to 94
const shouldRecurse = entry.name === 'node_modules' ||
entry.name.startsWith('@') ||
const shouldRecurse = entry.name === 'node_modules' ||
entry.name.startsWith('@') ||
entry.name === '.pnpm' || entry.name === '.yarn' ||
entry.name.startsWith('jest-runner@');
entry.name.startsWith('jest-runner@');
Copy link
Contributor

@claude claude bot Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Fixed in latest commit

@github-actions github-actions bot added the workflow-modified This PR modifies GitHub Actions workflows label Feb 16, 2026
@claude
Copy link
Contributor

claude bot commented Feb 16, 2026

PR Review Summary

Prek Checks

✅ All checks pass — ruff check and ruff format clean.

Mypy

⚠️ 13 pre-existing mypy errors in parse.py and test_runner.py (missing type annotations, generic type params, subprocess.run overload resolution). None introduced by this PR — the diff only removes lines and adds 4 lines of logging.

Code Review (Re-review)

Existing comments still open:

# File Issue Status
1 loop-runner.js:33 createRequire imported but unused after refactoring ⚠️ Still present
2 loop-runner.js:330,367 hasFailure and totalTimeMs declared but never read in runTests ⚠️ Still present
3 loop-runner.js:200-211 Fallback require('jest-runner') block doesn't detect Jest 29 — jestVersion stays 0, causing Jest 29 path to call undefined runTest ⚠️ Still present (bug)
4 test_runner.py:1050 result.stderr logged but always "" since line 1046 creates CompletedProcess(stderr="") ⚠️ Still present

No new critical issues found in the latest changes (c262f3c). The latest commit only merged main into the branch.

Test Coverage

File Stmts Miss Coverage
codeflash/languages/javascript/parse.py 255 131 49%
codeflash/languages/javascript/test_runner.py 521 193 63%

Analysis: The Python changes in this PR are minimal — removing a print statement, removing a log warning, and adding 4 lines of error-path logging. No new functionality requiring additional test coverage. The coverage numbers reflect the full file baselines, not a regression from this PR.

Test suite: 2374 passed, 8 failed (all in test_tracer.py — unrelated to this PR), 57 skipped.


Last updated: 2026-02-17T13:36Z

Saga4
Saga4 previously approved these changes Feb 16, 2026
if result.returncode != 0:
logger.info(f"Jest benchmarking failed with return code {result.returncode}")
logger.info(f"Jest benchmarking stdout: {result.stdout}")
logger.info(f"Jest benchmarking stderr: {result.stderr}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: result.stderr will always be "" here because line 1046 creates a new CompletedProcess with stderr="". The original stderr was merged into stdout on line 1043. This log line is misleading — it should either be removed or log the original stderr before it's merged.

Suggested change
logger.info(f"Jest benchmarking stderr: {result.stderr}")
logger.info(f"Jest benchmarking stderr (merged into stdout): {result.stderr}")

Or better yet, capture the original stderr before line 1046:

original_stderr = result.stderr
# ... line 1046 ...
logger.info(f"Jest benchmarking stderr: {original_stderr}")

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Copy link
Contributor

@Saga4 Saga4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can add unit tests or fix existing on path resolution and loop index

@mohammedahmed18
Copy link
Contributor Author

Can add unit tests or fix existing on path resolution and loop index

will add unit tests for the js pacakge

@mohammedahmed18 mohammedahmed18 merged commit f855c05 into main Feb 17, 2026
29 of 31 checks passed
@mohammedahmed18 mohammedahmed18 deleted the fix/jest30-pnpm-resolution branch February 17, 2026 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

workflow-modified This PR modifies GitHub Actions workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments