Skip to content

test_runner: support typescript module mocking - #54878

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
marco-ippolito:fix/mock-moduòe
Sep 19, 2024

Hidden character warning

The head ref may contain hidden characters: "fix/mock-modu\u00f2e"
Merged

test_runner: support typescript module mocking#54878
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
marco-ippolito:fix/mock-moduòe

Conversation

@marco-ippolito

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

Copy link
Copy Markdown
Member

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

Fixes: #54428

@nodejs-github-bot

Copy link
Copy Markdown
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
@github-actions

Copy link
Copy Markdown
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
Comment thread lib/internal/test_runner/mock/loader.js Outdated
@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
Copy Markdown
Collaborator

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

codecov Bot commented Sep 10, 2024

Copy link
Copy Markdown

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
Copy Markdown
Member Author

not sure if its a semver minor or patch

@cjihrig

cjihrig commented Sep 11, 2024

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
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
Copy Markdown
Collaborator

@cjihrig

cjihrig commented Sep 15, 2024

Copy link
Copy Markdown
Contributor

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

@marco-ippolito

Copy link
Copy Markdown
Member Author

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

@cjihrig

cjihrig commented Sep 18, 2024

Copy link
Copy Markdown
Contributor

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

@marco-ippolito

Copy link
Copy Markdown
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
Copy Markdown
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
@nodejs-github-bot

Copy link
Copy Markdown
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 <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
@aduh95 aduh95 mentioned this pull request Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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