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

test: add common.mustNotMutateObjectDeep() #43196

Merged
merged 2 commits into from
Jul 13, 2022
Merged
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
35 changes: 35 additions & 0 deletions test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,40 @@ If `fn` is not provided, an empty function will be used.
Returns a function that triggers an `AssertionError` if it is invoked. `msg` is
used as the error message for the `AssertionError`.

### `mustNotMutateObjectDeep([target])`

* `target` [\<any>][<any>] default = `undefined`
* return [\<any>][<any>]

If `target` is an Object, returns a proxy object that triggers
an `AssertionError` on mutation attempt, including mutation of deeply nested
Objects. Otherwise, it returns `target` directly.

Use of this function is encouraged for relevant regression tests.

```mjs
import { open } from 'node:fs/promises';
import { mustNotMutateObjectDeep } from '../common/index.mjs';

const _mutableOptions = { length: 4, position: 8 };
const options = mustNotMutateObjectDeep(_mutableOptions);

// In filehandle.read or filehandle.write, attempt to mutate options will throw
// In the test code, options can still be mutated via _mutableOptions
const fh = await open('/path/to/file', 'r+');
const { buffer } = await fh.read(options);
_mutableOptions.position = 4;
await fh.write(buffer, options);

// Inline usage
const stats = await fh.stat(mustNotMutateObjectDeep({ bigint: true }));
console.log(stats.size);
```

Caveats: built-in objects that make use of their internal slots (for example,
`Map`s and `Set`s) might not work with this function. It returns Functions
directly, not preventing their mutation.

### `mustSucceed([fn])`

* `fn` [\<Function>][<Function>] default = () => {}
Expand Down Expand Up @@ -1024,6 +1058,7 @@ See [the WPT tests README][] for details.
[<Function>]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function
[<Object>]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object
[<RegExp>]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp
[<any>]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Data_types
[<bigint>]: https://github.com/tc39/proposal-bigint
[<boolean>]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Boolean_type
[<number>]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type
Expand Down
47 changes: 47 additions & 0 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,52 @@ function mustNotCall(msg) {
};
}

const _mustNotMutateObjectDeepProxies = new WeakMap();

function mustNotMutateObjectDeep(original) {
// Return primitives and functions directly. Primitives are immutable, and
// proxied functions are impossible to compare against originals, e.g. with
// `assert.deepEqual()`.
if (original === null || typeof original !== 'object') {
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
return original;
}

const cachedProxy = _mustNotMutateObjectDeepProxies.get(original);
if (cachedProxy) {
return cachedProxy;
}

const _mustNotMutateObjectDeepHandler = {
__proto__: null,
defineProperty(target, property, descriptor) {
assert.fail(`Expected no side effects, got ${inspect(property)} ` +
'defined');
},
deleteProperty(target, property) {
assert.fail(`Expected no side effects, got ${inspect(property)} ` +
'deleted');
},
get(target, prop, receiver) {
return mustNotMutateObjectDeep(Reflect.get(target, prop, receiver));
},
preventExtensions(target) {
assert.fail('Expected no side effects, got extensions prevented on ' +
inspect(target));
},
set(target, property, value, receiver) {
assert.fail(`Expected no side effects, got ${inspect(value)} ` +
`assigned to ${inspect(property)}`);
},
setPrototypeOf(target, prototype) {
assert.fail(`Expected no side effects, got set prototype to ${prototype}`);
}
};

const proxy = new Proxy(original, _mustNotMutateObjectDeepHandler);
_mustNotMutateObjectDeepProxies.set(original, proxy);
return proxy;
}

function printSkipMessage(msg) {
console.log(`1..0 # Skipped: ${msg}`);
}
Expand Down Expand Up @@ -827,6 +873,7 @@ const common = {
mustCall,
mustCallAtLeast,
mustNotCall,
mustNotMutateObjectDeep,
mustSucceed,
nodeProcessAborted,
PIPE,
Expand Down
2 changes: 2 additions & 0 deletions test/common/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const {
canCreateSymLink,
getCallSite,
mustNotCall,
mustNotMutateObjectDeep,
printSkipMessage,
skip,
nodeProcessAborted,
Expand Down Expand Up @@ -81,6 +82,7 @@ export {
canCreateSymLink,
getCallSite,
mustNotCall,
mustNotMutateObjectDeep,
printSkipMessage,
skip,
nodeProcessAborted,
Expand Down
225 changes: 225 additions & 0 deletions test/parallel/test-common-must-not-mutate-object-deep.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
import { mustNotMutateObjectDeep } from '../common/index.mjs';
import assert from 'node:assert';
import { promisify } from 'node:util';

// Test common.mustNotMutateObjectDeep()

const original = {
foo: { bar: 'baz' },
qux: null,
quux: [
'quuz',
{ corge: 'grault' },
],
};

// Make a copy to make sure original doesn't get altered by the function itself.
const backup = structuredClone(original);

// Wrapper for convenience:
const obj = () => mustNotMutateObjectDeep(original);

function testOriginal(root) {
assert.deepStrictEqual(root, backup);
return root.foo.bar === 'baz' && root.quux[1].corge.length === 6;
}

function definePropertyOnRoot(root) {
Object.defineProperty(root, 'xyzzy', {});
}

function definePropertyOnFoo(root) {
Object.defineProperty(root.foo, 'xyzzy', {});
}

function deletePropertyOnRoot(root) {
delete root.foo;
}

function deletePropertyOnFoo(root) {
delete root.foo.bar;
}

function preventExtensionsOnRoot(root) {
Object.preventExtensions(root);
}

function preventExtensionsOnFoo(root) {
Object.preventExtensions(root.foo);
}

function preventExtensionsOnRootViaSeal(root) {
Object.seal(root);
}

function preventExtensionsOnFooViaSeal(root) {
Object.seal(root.foo);
}

function preventExtensionsOnRootViaFreeze(root) {
Object.freeze(root);
}

function preventExtensionsOnFooViaFreeze(root) {
Object.freeze(root.foo);
}

function setOnRoot(root) {
root.xyzzy = 'gwak';
}

function setOnFoo(root) {
root.foo.xyzzy = 'gwak';
}

function setQux(root) {
root.qux = 'gwak';
}

function setQuux(root) {
root.quux.push('gwak');
}

function setQuuxItem(root) {
root.quux[0] = 'gwak';
}

function setQuuxProperty(root) {
root.quux[1].corge = 'gwak';
}

function setPrototypeOfRoot(root) {
Object.setPrototypeOf(root, Array);
}

function setPrototypeOfFoo(root) {
Object.setPrototypeOf(root.foo, Array);
}

function setPrototypeOfQuux(root) {
Object.setPrototypeOf(root.quux, Array);
}


{
assert.ok(testOriginal(obj()));

assert.throws(
() => definePropertyOnRoot(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => definePropertyOnFoo(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => deletePropertyOnRoot(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => deletePropertyOnFoo(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => preventExtensionsOnRoot(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => preventExtensionsOnFoo(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => preventExtensionsOnRootViaSeal(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => preventExtensionsOnFooViaSeal(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => preventExtensionsOnRootViaFreeze(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => preventExtensionsOnFooViaFreeze(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => setOnRoot(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => setOnFoo(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => setQux(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => setQuux(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => setQuux(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => setQuuxItem(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => setQuuxProperty(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => setPrototypeOfRoot(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => setPrototypeOfFoo(obj()),
{ code: 'ERR_ASSERTION' }
);
assert.throws(
() => setPrototypeOfQuux(obj()),
{ code: 'ERR_ASSERTION' }
);

// Test that no mutation happened:
assert.ok(testOriginal(obj()));
}

// Test various supported types, directly and nested:
[
undefined, null, false, true, 42, 42n, Symbol('42'), NaN, Infinity, {}, [],
() => {}, async () => {}, Promise.resolve(), Math, Object.create(null),
].forEach((target) => {
assert.deepStrictEqual(mustNotMutateObjectDeep(target), target);
assert.deepStrictEqual(mustNotMutateObjectDeep({ target }), { target });
assert.deepStrictEqual(mustNotMutateObjectDeep([ target ]), [ target ]);
});

// Test that passed functions keep working correctly:
{
const fn = () => 'blep';
fn.foo = {};
const fnImmutableView = mustNotMutateObjectDeep(fn);
assert.deepStrictEqual(fnImmutableView, fn);

// Test that the function still works:
assert.strictEqual(fn(), 'blep');
assert.strictEqual(fnImmutableView(), 'blep');

// Test that the original function is not deeply frozen:
fn.foo.bar = 'baz';
assert.strictEqual(fn.foo.bar, 'baz');
assert.strictEqual(fnImmutableView.foo.bar, 'baz');

// Test the original function is not frozen:
fn.qux = 'quux';
assert.strictEqual(fn.qux, 'quux');
assert.strictEqual(fnImmutableView.qux, 'quux');

// Redefining util.promisify.custom also works:
promisify(mustNotMutateObjectDeep(promisify(fn)));
}
2 changes: 1 addition & 1 deletion test/parallel/test-fs-options-immutable.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const common = require('../common');
const fs = require('fs');
const path = require('path');

const options = Object.freeze({});
const options = common.mustNotMutateObjectDeep({});
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();

Expand Down
9 changes: 3 additions & 6 deletions test/parallel/test-https-agent-create-connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,19 +139,16 @@ function createServer() {
server.listen(0, common.mustCall(() => {
const port = server.address().port;
const host = 'localhost';
const options = {
const options = common.mustNotMutateObjectDeep({
port: 3000,
rejectUnauthorized: false
};
rejectUnauthorized: false,
});

const socket = agent.createConnection(port, host, options);
socket.on('connect', common.mustCall((data) => {
socket.end();
}));
socket.on('end', common.mustCall(() => {
assert.deepStrictEqual(options, {
port: 3000, rejectUnauthorized: false
});
server.close();
}));
}));
Expand Down
6 changes: 4 additions & 2 deletions test/parallel/test-https-argument-of-creating.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ const dftProtocol = {};

// Test for immutable `opts`
{
const opts = { foo: 'bar', ALPNProtocols: [ 'http/1.1' ] };
const opts = common.mustNotMutateObjectDeep({
foo: 'bar',
ALPNProtocols: [ 'http/1.1' ],
});
const server = https.createServer(opts);

tls.convertALPNProtocols([ 'http/1.1' ], dftProtocol);
assert.deepStrictEqual(opts, { foo: 'bar', ALPNProtocols: [ 'http/1.1' ] });
assert.strictEqual(server.ALPNProtocols.compare(dftProtocol.ALPNProtocols),
0);
}
Expand Down