Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add eval ESM module tests #27956

Closed
wants to merge 3 commits into from
Closed

test: add eval ESM module tests #27956

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a83525
test: add eval ESM module tests
zeckson May 29, 2019
72e8ac2
test: do not validate ESM warning
zeckson Jun 4, 2019
bed5887
test: validate import.meta and require
zeckson Jun 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions test/parallel/test-cli-eval.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,3 +229,48 @@ child.exec(`${nodejs} --use-strict -p process.execArgv`,
assert.strictEqual(err, null);
}));
});

// ESModule eval tests


// Assert that "42\n" is written to stdout on module eval.
const execOptions = '--experimental-modules --input-type module';
child.exec(
zeckson marked this conversation as resolved.
Show resolved Hide resolved
`${nodejs} ${execOptions} --eval "console.log(42)"`,
common.mustCall((err, stdout) => {
assert.ifError(err);
assert.strictEqual(stdout, '42\n');
}));

// Assert that "42\n" is written to stdout with print option.
child.exec(
`${nodejs} ${execOptions} --print --eval "42"`,
common.mustCall((err, stdout) => {
assert.ifError(err);
assert.strictEqual(stdout, '42\n');
}));

// Assert that error is written to stderr on invalid input.
child.exec(
`${nodejs} ${execOptions} --eval "!!!!"`,
common.mustCall((err, stdout, stderr) => {
assert.ok(err);
assert.strictEqual(stdout, '');
assert.ok(stderr.indexOf('SyntaxError: Unexpected end of input') > 0);
}));

// Assert that require is undefined in ESM support
child.exec(
`${nodejs} ${execOptions} --eval "console.log(typeof require);"`,
common.mustCall((err, stdout) => {
assert.ifError(err);
assert.strictEqual(stdout, 'undefined\n');
}));
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably don't need this test, but there's nothing wrong with including it.

Copy link
Contributor

Choose a reason for hiding this comment

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

It may be valuable to capture the default behavior. If we'd ever (accidentally) flip the default type, this would fail. Though realistically a lot of tests would fail then...


// Assert that import.meta is defined in ESM
child.exec(
`${nodejs} ${execOptions} --eval "console.log(typeof import.meta);"`,
Copy link
Contributor

Choose a reason for hiding this comment

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

It could also be worth making this a test of import.meta.url. Note that such a test would be the exact test needed to fix #28160 (comment).

Copy link
Contributor

Choose a reason for hiding this comment

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

I would be a bit concerned that it would increase the test scope. E.g. this test is meant to verify that the source is run as a module. If we change the exact string in import.meta.url in the future, it would break this test even though the source is still running as a module. If we want to test specific behavior ("relative imports work in --eval"), I would prefer a dedicated test for that.

common.mustCall((err, stdout) => {
assert.ifError(err);
assert.strictEqual(stdout, 'object\n');
}));