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.

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

Make tests engine agnostic #16272

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion test/parallel/test-buffer-slow.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ try {
assert.strictEqual(
SlowBuffer(buffer.kMaxLength).length, buffer.kMaxLength);
} catch (e) {
assert.strictEqual(e.message, 'Array buffer allocation failed');
// Don't match on message as it is from the JavaScript engine. V8 and
// ChakraCore provide different messages.
assert.strictEqual(e.name, 'RangeError');
}

// should work with number-coercible values
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-cli-syntax.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ const syntaxArgs = [
['--check']
];

const syntaxErrorRE = /^SyntaxError: Unexpected identifier$/m;
// Match on the name of the `Error` but not the message as it is different
// depending on the JavaScript engine.
const syntaxErrorRE = /^SyntaxError: \b/m;
const notFoundRE = /^Error: Cannot find module/m;

// test good syntax with and without shebang
Expand Down
8 changes: 5 additions & 3 deletions test/parallel/test-console-count.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,12 @@ assert.strictEqual(buf, 'default: 2\n');

process.stdout.write = stdoutWrite;

// Symbol labels do not work
// Symbol labels do not work. Only check that the `Error` is a `TypeError`. Do
// not check the message because it is different depending on the JavaScript
// engine.
assert.throws(
() => console.count(Symbol('test')),
/^TypeError: Cannot convert a Symbol value to a string$/);
TypeError);
assert.throws(
() => console.countReset(Symbol('test')),
/^TypeError: Cannot convert a Symbol value to a string$/);
TypeError);
6 changes: 4 additions & 2 deletions test/parallel/test-console.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@ assert.doesNotThrow(function() {
console.timeEnd('label');
});

// Check that the `Error` is a `TypeError` but do not check the message as it
// will be different in different JavaScript engines.
assert.throws(() => console.time(Symbol('test')),
/^TypeError: Cannot convert a Symbol value to a string$/);
TypeError);
assert.throws(() => console.timeEnd(Symbol('test')),
/^TypeError: Cannot convert a Symbol value to a string$/);
TypeError);


// an Object with a custom .inspect() function
Expand Down
8 changes: 2 additions & 6 deletions test/parallel/test-error-reporting.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,14 @@ function errExec(script, callback) {
assert.ok(err);

// More than one line of error output.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: should be "At least one line ... " or the assertion should be > 1.

assert.ok(stderr.split('\n').length > 2);

// Assert the script is mentioned in error output.
assert.ok(stderr.includes(script));
assert.ok(stderr.split('\n').length);

// Proxy the args for more tests.
callback(err, stdout, stderr);
});
}

const syntaxErrorMessage = /SyntaxError/;
const syntaxErrorMessage = /\bSyntaxError\b/;


// Simple throw error
Expand All @@ -65,7 +62,6 @@ errExec('throws_error3.js', common.mustCall(function(err, stdout, stderr) {

// throw ILLEGAL error
errExec('throws_error4.js', common.mustCall(function(err, stdout, stderr) {
assert.ok(/\/\*\*/.test(stderr));
assert.ok(syntaxErrorMessage.test(stderr));
}));

Expand Down
7 changes: 5 additions & 2 deletions test/parallel/test-http-outgoing-proto.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,14 @@ assert.throws(() => {
message: 'The first argument must be one of type string or buffer'
}));

// addTrailers
// addTrailers()
// The `Error` comes from the JavaScript engine so confirm that it is a
// `TypeError` but do not check the message. It will be different in different
// JavaScript engines.
assert.throws(() => {
const outgoingMessage = new OutgoingMessage();
outgoingMessage.addTrailers();
}, /^TypeError: Cannot convert undefined or null to object$/);
}, TypeError);

assert.throws(() => {
const outgoingMessage = new OutgoingMessage();
Expand Down
13 changes: 9 additions & 4 deletions test/parallel/test-internal-util-decorate-error-stack.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,16 @@ const obj = {};
decorateErrorStack(obj);
assert.strictEqual(obj.stack, undefined);

// Verify that the stack is decorated when possible
// Verify that the stack is decorated when possible.
function checkStack(stack) {
const matches = stack.match(/var foo bar;/g);
assert.strictEqual(Array.isArray(matches), true);
assert.strictEqual(matches.length, 1);
// Matching only on a minimal piece of the stack because the string will vary
// greatly depending on the JavaScript engine. V8 includes `;` because it
// displays the line of code (`var foo bar;`) that is causing a problem.
// ChakraCore does not display the line of code but includes `;` in the phrase
// `Expected ';' `.
assert.ok(/;/g.test(stack));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the best we can do?
Can we at least test that it's a multiline string?

Copy link
Member Author

Choose a reason for hiding this comment

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

@refack I guess that's something. I've added this:

  // Test that it's a multiline string.
  assert.ok(/\n/g.test(stack));

Copy link
Contributor

Choose a reason for hiding this comment

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

💯

// Test that it's a multiline string.
assert.ok(/\n/g.test(stack));
}
let err;
const badSyntaxPath =
Expand Down
9 changes: 3 additions & 6 deletions test/parallel/test-os-eol.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,9 @@ const eol = common.isWindows ? '\r\n' : '\n';

assert.strictEqual(os.EOL, eol);

common.expectsError(function() {
os.EOL = 123;
}, {
type: TypeError,
message: /^Cannot assign to read only property 'EOL' of object '#<Object>'$/
});
// Test that the `Error` is a `TypeError` but do not check the message as it
// varies between different JavaScript engines.
assert.throws(function() { os.EOL = 123; }, TypeError);

const foo = 'foo';
Object.defineProperties(os, {
Expand Down
9 changes: 6 additions & 3 deletions test/parallel/test-process-env-symbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,23 @@ require('../common');

const assert = require('assert');
const symbol = Symbol('sym');
const errRegExp = /^TypeError: Cannot convert a Symbol value to a string$/;

// Verify that getting via a symbol key returns undefined.
assert.strictEqual(process.env[symbol], undefined);

// Verify that assigning via a symbol key throws.
// The message depends on the JavaScript engine and so will be different between
// different JavaScript engines. Confirm that the `Error` is a `TypeError` only.
assert.throws(() => {
process.env[symbol] = 42;
}, errRegExp);
}, TypeError);

// Verify that assigning a symbol value throws.
// The message depends on the JavaScript engine and so will be different between
// different JavaScript engines. Confirm that the `Error` is a `TypeError` only.
assert.throws(() => {
process.env.foo = symbol;
}, errRegExp);
}, TypeError);

// Verify that using a symbol with the in operator returns false.
assert.strictEqual(symbol in process.env, false);
Expand Down
14 changes: 8 additions & 6 deletions test/parallel/test-querystring-escape.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ assert.strictEqual(
'test'
);

// toString is not callable, must throw an error
assert.throws(() => qs.escape({ toString: 5 }),
/^TypeError: Cannot convert object to primitive value$/);
// `toString` is not callable, must throw an error.
// Error message will vary between different JavaScript engines, so only check
// that it is a `TypeError`.
assert.throws(() => qs.escape({ toString: 5 }), TypeError);

// should use valueOf instead of non-callable toString
// Should use valueOf instead of non-callable toString.
assert.strictEqual(qs.escape({ toString: 5, valueOf: () => 'test' }), 'test');

assert.throws(() => qs.escape(Symbol('test')),
/^TypeError: Cannot convert a Symbol value to a string$/);
// Error message will vary between different JavaScript engines, so only check
// that it is a `TypeError`.
assert.throws(() => qs.escape(Symbol('test')), TypeError);
4 changes: 3 additions & 1 deletion test/parallel/test-repl-harmony.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ const args = ['-i'];
const child = spawn(process.execPath, args);

const input = '(function(){"use strict"; const y=1;y=2})()\n';
const expectOut = /^> TypeError: Assignment to constant variable\.\n/;
// This message will vary based on JavaScript engine, so don't check the message
// contents beyond confirming that the `Error` is a `TypeError`.
const expectOut = /^> TypeError: /;

child.stderr.setEncoding('utf8');
child.stderr.on('data', function(c) {
Expand Down
7 changes: 6 additions & 1 deletion test/parallel/test-repl-syntax-error-stack.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ process.on('exit', () => {
});

common.ArrayStream.prototype.write = function(output) {
if (/var foo bar;/.test(output))
// Matching only on a minimal piece of the stack because the string will vary
// greatly depending on the JavaScript engine. V8 includes `;` because it
// displays the line of code (`var foo bar;`) that is causing a problem.
// ChakraCore does not display the line of code but includes `;` in the phrase
// `Expected ';' `.
if (/;/.test(output))
found = true;
};

Expand Down
31 changes: 14 additions & 17 deletions test/parallel/test-repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function clean_up() {
function strict_mode_error_test() {
send_expect([
{ client: client_unix, send: 'ref = 1',
expect: /^ReferenceError:\sref\sis\snot\sdefined\nnode via Unix socket> $/ },
expect: /^ReferenceError:\s/ },
]);
}

Expand Down Expand Up @@ -143,7 +143,7 @@ function error_test() {
expect: prompt_unix },
// But passing the same string to eval() should throw
{ client: client_unix, send: 'eval("function test_func() {")',
expect: /^SyntaxError: Unexpected end of input/ },
expect: /^SyntaxError: / },
// Can handle multiline template literals
{ client: client_unix, send: '`io.js',
expect: prompt_multiline },
Expand Down Expand Up @@ -172,51 +172,48 @@ function error_test() {
// invalid input to JSON.parse error is special case of syntax error,
// should throw
{ client: client_unix, send: 'JSON.parse(\'{invalid: \\\'json\\\'}\');',
expect: /^SyntaxError: Unexpected token i/ },
expect: /^SyntaxError: / },
// end of input to JSON.parse error is special case of syntax error,
// should throw
{ client: client_unix, send: 'JSON.parse(\'066\');',
expect: /^SyntaxError: Unexpected number/ },
expect: /^SyntaxError: / },
// should throw
{ client: client_unix, send: 'JSON.parse(\'{\');',
expect: /^SyntaxError: Unexpected end of JSON input/ },
expect: /^SyntaxError: / },
// invalid RegExps are a special case of syntax error,
// should throw
{ client: client_unix, send: '/(/;',
expect: /^SyntaxError: Invalid regular expression:/ },
expect: /^SyntaxError: / },
// invalid RegExp modifiers are a special case of syntax error,
// should throw (GH-4012)
{ client: client_unix, send: 'new RegExp("foo", "wrong modifier");',
expect: /^SyntaxError: Invalid flags supplied to RegExp constructor/ },
expect: /^SyntaxError: / },
// strict mode syntax errors should be caught (GH-5178)
{ client: client_unix,
send: '(function() { "use strict"; return 0755; })()',
expect: /\bSyntaxError: Octal literals are not allowed in strict mode/ },
expect: /\bSyntaxError: / },
{
client: client_unix,
send: '(function(a, a, b) { "use strict"; return a + b + c; })()',
expect:
/\bSyntaxError: Duplicate parameter name not allowed in this context/
expect: /\bSyntaxError: /
},
{
client: client_unix,
send: '(function() { "use strict"; with (this) {} })()',
expect: /\bSyntaxError: Strict mode code may not include a with statement/
expect: /\bSyntaxError: /
},
{
client: client_unix,
send: '(function() { "use strict"; var x; delete x; })()',
expect:
/\bSyntaxError: Delete of an unqualified identifier in strict mode/
expect: /\bSyntaxError: /
},
{ client: client_unix,
send: '(function() { "use strict"; eval = 17; })()',
expect: /\bSyntaxError: Unexpected eval or arguments in strict mode/ },
expect: /\bSyntaxError: / },
{
client: client_unix,
send: '(function() { "use strict"; if (true) function f() { } })()',
expect:
/\bSyntaxError: In strict mode code, functions can only be declared at top level or inside a block\./
expect: /\bSyntaxError: /
},
// Named functions can be used:
{ client: client_unix, send: 'function blah() { return 1; }',
Expand Down Expand Up @@ -268,7 +265,7 @@ function error_test() {
expect: `Invalid REPL keyword\n${prompt_unix}` },
// fail when we are not inside a String and a line continuation is used
{ client: client_unix, send: '[] \\',
expect: /\bSyntaxError: Invalid or unexpected token/ },
expect: /\bSyntaxError: / },
// do not fail when a String is created with line continuation
{ client: client_unix, send: '\'the\\\nfourth\\\neye\'',
expect: `${prompt_multiline}${prompt_multiline}'thefourtheye'\n${
Expand Down
5 changes: 2 additions & 3 deletions test/parallel/test-require-json.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ try {
require(fixtures.path('invalid.json'));
} catch (err) {
assert.ok(
/test[/\\]fixtures[/\\]invalid\.json: Unexpected string/.test(err.message),
`require() json error should include path - currently: ${err.message}`
);
/test[/\\]fixtures[/\\]invalid\.json: /.test(err.message),
`require() json error should include path: ${err.message}`);
}
4 changes: 2 additions & 2 deletions test/parallel/test-tls-external-accessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ const tls = require('tls');
{
const pctx = tls.createSecureContext().context;
const cctx = Object.create(pctx);
assert.throws(() => cctx._external, /incompatible receiver/);
assert.throws(() => cctx._external, TypeError);
pctx._external;
}
{
const pctx = tls.createSecurePair().credentials.context;
const cctx = Object.create(pctx);
assert.throws(() => cctx._external, /incompatible receiver/);
assert.throws(() => cctx._external, TypeError);
pctx._external;
}