Skip to content

esm: mordernise old tests - #50639

Closed
JakobJingleheimer wants to merge 1 commit into
nodejs:mainfrom
JakobJingleheimer:test/modernise-old-esm-tests
Closed

esm: mordernise old tests#50639
JakobJingleheimer wants to merge 1 commit into
nodejs:mainfrom
JakobJingleheimer:test/modernise-old-esm-tests

Conversation

@JakobJingleheimer

Copy link
Copy Markdown
Member

My goal is to improve DX and performance. The way old tests were written makes them very difficult to figure out what they're even asserting and, when they're failing, why.

Converting them to node's test runner in concurrent mode should on its own improve run-time (the first test I ported, which is quite a small test, improved its run-time by ~10%).

@nodejs-github-botnodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Nov 9, 2023
@JakobJingleheimer
JakobJingleheimerforce-pushed the test/modernise-old-esm-tests branch from 7df3076 to 44abe5bCompareNovember 9, 2023 17:21
Comment on lines +48 to +56
try {
require('../fixtures/es-modules/package-type-module/index.js');
assert.fail('Expected CJS to fail loading from type: module package.');
} catch (e) {
assert.match(e.toString(), /require\(\) of ES Module/g);
assert.match(e.message, /require\(\) of ES Module/g);
assert.strictEqual(e.code, 'ERR_REQUIRE_ESM');
assert.strictEqual(e.name, 'Error');
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's use assert.throws, no?

This was referenced Aug 13, 2024
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

esmIssues and PRs related to the ECMAScript Modules implementation.needs-ciPRs that need a full CI run.staletestIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@JakobJingleheimer@aduh95@nodejs-github-bot@avivkeller