test: migrate message source map tests from Python to JS#49238
Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom Aug 29, 2023
Merged
Conversation
MoLow
approved these changes
Aug 19, 2023
Member
|
Thanks for this work! I don't think we need to make such an effort for git to detect these as renames as long as reviewing these PRs makes sense and doesn't include hundreds of files :) also, please fix lint errors and amend the commit message for it to comply with guidlines |
Contributor
Author
|
Thanks for your review. I have updated the commit message and the code according to Lint. |
46d42ca to
bee982b
Compare
Member
|
commits still don't adhere guidelines |
0a273de to
e154b45
Compare
Migrate the remaining source map tests in the `test/message` folder from Python to JS. Fixes: nodejs#47707 test: migrate message source map tests from Python to JS
e154b45 to
0991cb3
Compare
Contributor
Author
|
I squashed my commits, hope it works this time. |
MoLow
approved these changes
Aug 27, 2023
Collaborator
25 tasks
Contributor
Author
|
Hi @MoLow, how can I pass the failed tests? |
21 tasks
Collaborator
Member
I ran a rebuild |
Collaborator
Collaborator
Collaborator
|
Landed in 9cd70f4 |
Member
|
@yiyunlei thanks for your contribution! |
Contributor
Author
|
@MoLow Thanks for your review! |
This was referenced Aug 30, 2023
Merged
alexfernandez
pushed a commit
to alexfernandez/node
that referenced
this pull request
Nov 1, 2023
Migrate the remaining source map tests in the `test/message` folder from Python to JS. Fixes: nodejs#47707 test: migrate message source map tests from Python to JS PR-URL: nodejs#49238 Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
sercher
added a commit
to sercher/graaljs
that referenced
this pull request
Apr 25, 2024
Migrate the remaining source map tests in the `test/message` folder from Python to JS. Fixes: nodejs/node#47707 test: migrate message source map tests from Python to JS PR-URL: nodejs/node#49238 Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
sercher
added a commit
to sercher/graaljs
that referenced
this pull request
Apr 25, 2024
Migrate the remaining source map tests in the `test/message` folder from Python to JS. Fixes: nodejs/node#47707 test: migrate message source map tests from Python to JS PR-URL: nodejs/node#49238 Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some JS files are detected as deleted and newly created even though they are very similar. I also mentioned the reason here #47707 (comment). IMHO, it is because the default similarity index for
git diffis 50%.Same for most
.snapshotfiles ( they are also detected as deleted ), the main difference between the original.outfile and the.snapshotfile is.snapshotfiles have more details likeObject.<anonymous>.Example:
node/test/fixtures/source-map/output/source_map_throw_catch.snapshotnode/test/message/source_map_throw_catch.outShould I replace these
Object.<anonymous>?Also, #47707 (comment)