Skip to content

test: fix debug test crashes caused by sea tests #60807

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

test: fix debug test crashes caused by sea tests #60807

wants to merge 1 commit into from
+17 −1

Conversation

@vmoroz
Copy link
Member

@vmoroz vmoroz commented Nov 22, 2025

When we compile for debug on Windows we have the tests crashing with the error log shown below.
The root cause of the issue is that when the SEA tests are probing for parallel execution they call context.GetVm('none', 'none').
This call results with trying to use Release\node.exe that is not created for debug-only builds.

In this PR we record the first --mode passed to tools\test.py and then we use it as a fallback when the context.GetVm('none', 'none') raises an exception.

Since I do not know the full background why we use none/none and why GetVm unconditionally returns the Release path in that case, this is the best solution that I could come with. Feel free to propose a better version.

This PR and and PR #60806 finally unblock the test runs on Windows while working on the code.

The error log being addressed by this PR:

Node.js v26.0.0-pre
running 'python tools\test.py --mode=debug  js-native-api'
Traceback (most recent call last):
  File "E:\GitHub\nodejs\node\tools\test.py", line 1854, in <module>
    sys.exit(Main())
             ^^^^^^
  File "E:\GitHub\nodejs\node\tools\test.py", line 1707, in Main
    root.GetTestStatus(context, sections, defs)
  File "E:\GitHub\nodejs\node\tools\test.py", line 942, in GetTestStatus
    tests_repos.GetTestStatus(context, sections, defs)
  File "E:\GitHub\nodejs\node\tools\test.py", line 909, in GetTestStatus
    self.GetConfiguration(context).GetTestStatus(sections, defs)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "E:\GitHub\nodejs\node\tools\test.py", line 891, in GetConfiguration
    self.config = module.GetConfiguration(context, self.path)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "E:\GitHub\nodejs\node\test\sea\testcfg.py", line 8, in GetConfiguration
    vm = context.GetVm('none', 'none')
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "E:\GitHub\nodejs\node\tools\test.py", line 991, in GetVm
    raise ValueError('Could not find executable. Should be ' + name)
ValueError: Could not find executable. Should be E:\GitHub\nodejs\node\Release\node.exe

@vmoroz vmoroz requested a review from joyeecheung November 22, 2025 00:55
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Nov 22, 2025

Review requested:

  • @nodejs/single-executable

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Nov 22, 2025
@vmoroz vmoroz mentioned this pull request Nov 22, 2025
Open
@codecov
Copy link

codecov bot commented Nov 22, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.56%. Comparing base (93c2581) to head (11fa798).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #60807      +/-   ##
==========================================
+ Coverage   88.43%   88.56%   +0.12%     
==========================================
  Files         703      703              
  Lines      208235   208254      +19     
  Branches    40085    40176      +91     
==========================================
+ Hits       184162   184430     +268     
+ Misses      16069    15841     -228     
+ Partials     8004     7983      -21

see 45 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joyeecheung joyeecheung Awaiting requested review from joyeecheung

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vmoroz @nodejs-github-bot