test: refactor test-runner-module-mocking
#54233
Conversation
|
Review requested:
|
nodejs-github-bot
added
needs-ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #54233 +/- ##
==========================================
- Coverage 87.10% 87.10% -0.01%
==========================================
Files 647 647
Lines 181739 181739
Branches 34887 34886 -1
==========================================
- Hits 158310 158301 -9
- Misses 16738 16740 +2
- Partials 6691 6698 +7 |
| cjsImpl = require(cjsFixture); | ||
|
|
||
| assert.strictEqual(esmImpl.default.string, 'original cjs string'); | ||
| assert.strictEqual(cjsImpl.string, 'original cjs string'); | ||
| }); | ||
|
|
||
| test('ESM mocks can be used by both module systems', async (t) => { |
There was a problem hiding this comment.
Why remove this test? (Other changes LGTM)
There was a problem hiding this comment.
I couldn't find a way to refactor it. Either we use an absolute path for require and a file: URL for import() (but that's already the previous test), either we use the same relative specifier (but that's already the following test); so it seemed to me it was testing a "real" use case, or rather it's testing some particular case where an absolute path coincides with an origin-relative URL – but we cannot guarantee that, it depends on where the node repo was cloned.
aduh95
force-pushed
the
refactor-mock_loader-test
branch
from
90b5261
to
8ef4cb9
Compare
github-actions
bot
removed
the
request-ci
aduh95
added
the
author ready
aduh95
added
the
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in 9e6c526 |
PR-URL: #54233 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Trying to use paths as import specifiers can lead to weird results if the path contain special URL chars (e.g.
#,?,%).