Repository navigation
message tests can be migrated from python to JS #47707
Description
Activity
@RafaelGSS is this a good candidate for Grace Hopper Open Source Day?
I would like to work on this if is available.
@projectnoa no need to ask, this is an issue that many people can work on since there are many tests under
tests/message.Reacted by Juan and Kabir Baidhya@projectnoa no need to ask, this is an issue that many people can work on since there are many tests under
tests/message.Yes. Please follow the example of #47498, and branch off of that PR’s branch so you get the new helpers that it adds. You might also want to add a comment on this thread letting others know which tests you plan to migrate, so that no one else works on the same ones while you do, duplicating effort.
Reacted by Moshe Atlow, Juan and RossieReeGreat! I'll dig into the project and mention what I'll tackle tomorrow.
Tests I will be working on:
assert_throws_stack.js,
assert_throws_stack.out,
core_line_numbers.js,
core_line_numbers.out,
eval_messages.js,
eval_messages.out,
if-error-has-good-stack.js,
if-error-has-good-stack.out,
internal_assert.js,
internal_assert.out,
internal_assert_fail.js,
internal_assert_fail.out,
max_tick_depth.js,
max_tick_depth.out,
message.status,
nexttick_throw.js,
nexttick_throw.out,
promise_unhandled_warn_with_error.js,
promise_unhandled_warn_with_error.out,
source_map_enclosing_function.js,
source_map_enclosing_function.out,
source_map_reference_error_tabs.js,
source_map_reference_error_tabs.out,
source_map_sourcemapping_url_string.js,
source_map_sourcemapping_url_string.out,
source_map_throw_catch.js,
source_map_throw_catch.out,
source_map_throw_icu.js,
source_map_throw_icu.out,
source_map_throw_set_immediate.js,
source_map_throw_set_immediate.out,
stdin_messages.js,
stdin_messages.out,
test-no-extra-info-on-fatal-exception.js,
test-no-extra-info-on-fatal-exception.out,
testcfg.pyReacted by Geoffrey Booth and SURAJ KUMAR SHARMA@projectnoa we try to avoid cookie licking, so if you have a ready PR just open one or multiple PRs
Reacted by Juan@MoLow I would love to help too
@mertcanaltin go for it!
Reacted by Mert Can AltinApologies @MoLow, you were right. I didn't do my due diligence and jumped into the task without understanding the challenge ahead. I still want to do the work and contribute but I need to ask some questions.
To my understanding, there's a set of unit tests in Python that need to be migrated to JS. I'm having trouble finding the Python tests. Can someone point me in the right direction?
These tests will be residing in the test/message directory, correct? Are the message tests targeting a specific module or feature?
What are the .out files in this directory?
Also, the tests files I announced that I would work on are not the target of this operation. I should have paid attention a notice that they were tests already migrated. Apologies about that. I will instead focus on understanding the test workflow and see how I can start migrating a few tests.
I was able to compile and run all current tests successfully, so I'm ready to work as soon as I can understand the test better.
Thanks!
@projectnoa Once you’ve checked out the
noderepo and built, runtools/test.py -J --mode=release message
To run all the
messagetests. At the moment, now that #47498 has landed there are only 26 left.These aren’t tests written in Python; they’re tests that are run by the Python script
tools/test.py, but in a special way different from all the other tests in the Node codebase. The Python code runs all the.jsfiles withintest/messageand compares their outputs against the matching.outfiles in the same folder, using a complicated regex pattern. Whenever the regex comparison fails, the test fails.We don’t want to have all this logic in Python, so we’ve been migrating the
test/messagetests to use a new pattern where the regex comparison is done in JavaScript; the #47498 PR created a new helper to do that comparison, and migrated a few dozen tests to use it. The work that remains is to migrate the rest of themessagetests, following the example of #47498.Reacted by Moshe Atlow, Sankalp and Mert Can AltinReacted by Juan, Mert Can Altin, Yiyun Lei and Benyaminmahdavi- added a commit that references this issue
on May 5, 2023 75 remaining items
- added a commit that references this issue
on Nov 11, 2023 - added 2 commits that reference this issue
on Nov 23, 2023 - added a commit that references this issue
on Dec 11, 2023 - added 2 commits that reference this issue
on Apr 25, 2024 - added a commit that references this issue
on Apr 18, 2026 This issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 240 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.
See #47498 for an example of such a migration
pseudo-ttytests can migrate as well, but that will require some more work using something like https://github.com/microsoft/node-pty or similar