esm: modernize old tests

[puskin94]

Doing a round of fast cleanup of old open and stalled PRs.
Making sure this PR is moving along addressing the comments in the original discussion.
I added the author of the original PR in the commit message.

Co-Authored-By: Jacob Smith

[aduh95]

Sorry I forgot to ask you that earlier, but the subsystem is actually wrong since it only touches the test/ folder. It should be test: refactor test-esm-type-field-errors or something like that.

The instructions on how to do it:

# first make sure you are on top of the right commit
git fetch https://github.com/nodejs/node.git bade48fc9662dd0a63f88a8a12d0c824f435276d
git reset FETCH_HEAD --hard

# amend the commit message
git commit --amend

# Force push to your branch
git push origin HEAD:modernize-old-esm-tests --force-with-lease

Let me know if you prefer I do that for you.

[aduh95]

You might want to reopen the other PR btw, so the approval and the wait time do not reset. Let me know if you want me to push the correct commit over there instead. But it's also fine to use a new PR if that's what you prefer.

[puskin94]

@aduh95 no worries!
The process of amending a commit is (kinda) clear in my head, it is not the first time I do it!
Somehow in the previous PR the git history was completely fine everywhere locally but not in the "interactive amend" file, it was just showing me 8 commits over the 12 that were pushed. I also asked around and nobody was able to come up with a decent solution, so I decided to create a new PR instead of wasting my time :)
No worries, this MR is nor critical, urgent or important, it can wait 1-2 days :)

Thanks for your support btw!

[aduh95]

No issues, it's probably my bad, I supsect my force-push to your branch might have caused the issues you were seeing. FWIW git reset FETCH_HEAD --hard is often the solution :)

Sorry for the bike shedding, but test: modernize old tests feels too broad for the diff, because we're only updating one test file. IMO test: refactor `test-esm-type-field-errors` would be a better commit title.

[puskin94]

@aduh95 no worries! Changed the commit message, again :)

and about git reset FETCH_HEAD --hard: I avoid it as much as I can, so much that I prefer to create a new PR 😄

[aduh95]

Apologies again, I realize only now there's indeed a copy paste issue with Jacob's email:

node/README.md

Line 356 in a3ff3e8

**Jacob Smith** <<[email protected]>> (he/him)

I feel bad asking you again for a force push, but hopefully this time it's the last one: can you please add the missing e to Jacob's email? Or let me know if you prefer I do that for you.

[puskin94]

I knew I was right :) no worries, pushed again!

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/61111/

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.09%. Comparing base (6051826) to head (f3a94cd).
Report is 215 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54368      +/-   ##
==========================================
- Coverage   87.10%   87.09%   -0.02%     
==========================================
  Files         648      648              
  Lines      182216   182217       +1     
  Branches    34966    34955      -11     
==========================================
- Hits       158720   158694      -26     
- Misses      16787    16818      +31     
+ Partials     6709     6705       -4     

see 29 files with indirect coverage changes

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/61128/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/61143/

[puskin94]

is anything missing from this PR? could it get merged?

[nodejs-github-bot]

Landed in e4fdd0b