Skip to content

Conversation

marco-ippolito
Copy link
Member

@marco-ippolito marco-ippolito commented Sep 10, 2024

Ugh I named the branch with ò character and GitHub complains :(

Fixes: #54428

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/test_runner
  • @nodejs/typescript

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Sep 10, 2024
@jasnell jasnell added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 10, 2024
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Sep 10, 2024
Copy link
Contributor

Failed to start CI
- Validating Jenkins credentials
✔  Jenkins credentials valid
- Starting PR CI job
✘  Failed to start PR CI: 400 Bad Request
https://github.com/nodejs/node/actions/runs/10796555755

@marco-ippolito marco-ippolito added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Sep 10, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 10, 2024
@nodejs-github-bot
Copy link
Collaborator

@avivkeller avivkeller added the strip-types Issues or PRs related to strip-types support label Sep 10, 2024
Copy link

codecov bot commented Sep 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.02%. Comparing base (5de919b) to head (87c6d5e).
Report is 578 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #54878      +/-   ##
==========================================
+ Coverage   87.89%   88.02%   +0.13%     
==========================================
  Files         651      652       +1     
  Lines      183364   183764     +400     
  Branches    35711    35865     +154     
==========================================
+ Hits       161167   161766     +599     
+ Misses      15466    15244     -222     
- Partials     6731     6754      +23     
Files with missing lines Coverage Δ
lib/internal/modules/esm/translators.js 92.42% <ø> (+0.72%) ⬆️
lib/internal/test_runner/mock/loader.js 95.52% <100.00%> (ø)
lib/internal/test_runner/mock/mock.js 99.25% <100.00%> (+<0.01%) ⬆️

... and 101 files with indirect coverage changes

@marco-ippolito marco-ippolito added semver-minor PRs that contain new features and should be released in the next minor version. and removed semver-minor PRs that contain new features and should be released in the next minor version. labels Sep 11, 2024
@marco-ippolito
Copy link
Member Author

not sure if its a semver minor or patch

@cjihrig
Copy link
Contributor

cjihrig commented Sep 11, 2024

I think this is semver patch. Plus these are all experimental features anyway.

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 11, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 11, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator


test('logger', async () => {

const { logger: mockedModule} = await import('./module-logger.ts');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const { logger: mockedModule} = await import('./module-logger.ts');
const { logger: mockedModule } = await import('./module-logger.ts');

@nodejs-github-bot
Copy link
Collaborator

@cjihrig
Copy link
Contributor

cjihrig commented Sep 15, 2024

It looks like there is a relevant failure in the CI.

@marco-ippolito
Copy link
Member Author

I think its related to #54645 since it only happens on windows arm64 😭

@cjihrig
Copy link
Contributor

cjihrig commented Sep 18, 2024

Should we skip on that platform? For the purposes of module mocking I think that would be adequate.

@marco-ippolito
Copy link
Member Author

Should we skip on that platform? For the purposes of module mocking I think that would be adequate.

sure, I added the skip on windows arm on the test

@marco-ippolito marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 19, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 19, 2024
@nodejs-github-bot
Copy link
Collaborator

@marco-ippolito marco-ippolito added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 19, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 19, 2024
@nodejs-github-bot nodejs-github-bot merged commit bb40521 into nodejs:main Sep 19, 2024
55 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in bb40521

targos pushed a commit that referenced this pull request Oct 4, 2024
PR-URL: #54878
Fixes: #54428
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Erick Wendel <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
@aduh95 aduh95 mentioned this pull request Oct 9, 2024
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
PR-URL: nodejs#54878
Fixes: nodejs#54428
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Erick Wendel <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
@marco-ippolito marco-ippolito added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Nov 16, 2024
tpoisseau pushed a commit to tpoisseau/node that referenced this pull request Nov 21, 2024
PR-URL: nodejs#54878
Fixes: nodejs#54428
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Erick Wendel <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. strip-types Issues or PRs related to strip-types support test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

type stripping + module mocking causes test runner to throw