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: increase coverage of async_hooks #13336

Closed
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
8 changes: 4 additions & 4 deletions lib/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,15 @@ class AsyncResource {
}
this[trigger_id_symbol] = triggerId;

// Return immediately if there's nothing to do.
if (async_hook_fields[kInit] === 0)
return;

if (typeof type !== 'string' || type.length <= 0)
throw new TypeError('type must be a string with length > 0');
if (!Number.isSafeInteger(triggerId) || triggerId < 0)
throw new RangeError('triggerId must be an unsigned integer');

// Return immediately if there's nothing to do.
if (async_hook_fields[kInit] === 0)
return;

processing_hook = true;
for (var i = 0; i < active_hooks_array.length; i++) {
if (typeof active_hooks_array[i][init_symbol] === 'function') {
Expand Down
56 changes: 56 additions & 0 deletions test/async-hooks/test-callback-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const spawnSync = require('child_process').spawnSync;
const async_hooks = require('async_hooks');
const initHooks = require('./init-hooks');

switch (process.argv[2]) {
case 'test_init_callback':
initHooks({
oninit: common.mustCall(() => { throw new Error('test_init_callback'); })
Copy link
Contributor

Choose a reason for hiding this comment

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

There's the problem where if test_init_callback is never hit then the common.mustCall() will never run. Instead you'll need to do it like this:

const oninitMustCall = common.mustCall(() => { throw new Error('test_init_callback'); });
switch (process.argv[2]) {
  case 'test_init_callback':
    initHooks({ oninit: oninitMustCall }).enable();

Copy link
Member Author

Choose a reason for hiding this comment

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

😕 But IMO the actual intention here is just to ensure that the common.mustCall() will run only when the test_init_callback case is hit? Every switch case will be hit in different process because of spawnSync()s below.

Copy link
Contributor

Choose a reason for hiding this comment

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

But IMO the actual intention here is just to ensure that the common.mustCall() will run only when the test_init_callback case is hit?

Whoops. Missed that. You're correct.

}).enable();

async_hooks.emitInit(async_hooks.currentId(), 'test_init_callback_type',
async_hooks.triggerId());
break;
case 'test_callback':
initHooks({
onbefore: common.mustCall(() => { throw new Error('test_callback'); })
}).enable();

async_hooks.emitInit(async_hooks.currentId(), 'test_callback_type',
async_hooks.triggerId());
async_hooks.emitBefore(async_hooks.currentId());
break;
}

if (process.execArgv.includes('--abort-on-uncaught-exception')) {
initHooks({
oninit: common.mustCall(() => { throw new Error('test_callback_abort'); })
}).enable();

async_hooks.emitInit(async_hooks.currentId(), 'test_callback_abort',
async_hooks.triggerId());
}

const c1 = spawnSync(`${process.execPath}`, [__filename, 'test_init_callback']);
assert.strictEqual(c1.stderr.toString().split('\n')[0],
'Error: test_init_callback');
assert.strictEqual(c1.status, 1);

const c2 = spawnSync(`${process.execPath}`, [__filename, 'test_callback']);
assert.strictEqual(c2.stderr.toString().split('\n')[0], 'Error: test_callback');
assert.strictEqual(c2.status, 1);

const c3 = spawnSync(`${process.execPath}`, ['--abort-on-uncaught-exception',
__filename,
'test_callback_abort']);
assert.strictEqual(c3.stdout.toString(), '');

const stderrOutput = c3.stderr.toString()
.trim()
.split('\n')
.map((s) => s.trim());
assert.strictEqual(stderrOutput[0], 'Error: test_callback_abort');
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ const { checkInvocations } = require('./hook-checks');
const hooks = initHooks();
hooks.enable();

assert.throws(() => new AsyncResource(),
/^TypeError: type must be a string with length > 0$/);
assert.throws(() => new AsyncResource('invalid_trigger_id', null),
/^RangeError: triggerId must be an unsigned integer$/);

assert.strictEqual(typeof new AsyncResource('default_trigger_id').triggerId(),
'number');

// create first custom event 'alcazares' with triggerId derived
// from async_hooks currentId
const alcaTriggerId = async_hooks.currentId();
Expand All @@ -26,6 +34,10 @@ assert.strictEqual(typeof alcazares.uid, 'number');
assert.strictEqual(alcazares.triggerId, alcaTriggerId);
checkInvocations(alcazares, { init: 1 }, 'alcazares constructed');

assert.strictEqual(typeof alcaEvent.asyncId(), 'number');
assert.notStrictEqual(alcaEvent.asyncId(), alcaTriggerId);
assert.strictEqual(alcaEvent.triggerId(), alcaTriggerId);

alcaEvent.emitBefore();
checkInvocations(alcazares, { init: 1, before: 1 },
'alcazares emitted before');
Expand Down
46 changes: 46 additions & 0 deletions test/async-hooks/test-emit-before-after.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const spawnSync = require('child_process').spawnSync;
const async_hooks = require('async_hooks');
const initHooks = require('./init-hooks');

switch (process.argv[2]) {
case 'test_invalid_async_id':
async_hooks.emitBefore(-1);
Copy link
Contributor

Choose a reason for hiding this comment

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

To make sure these run I' possibly do this:

const emitBeforeMustCall = common.mustCall(() => async_hooks.emitBefore(-1));
switch (process.argv[2]) {
  case 'test_invalid_async_id':
    emitBeforeMustCall();

break;
case 'test_invalid_trigger_id':
async_hooks.emitBefore(1, -1);
break;
}

const c1 = spawnSync(process.execPath, [__filename, 'test_invalid_async_id']);
assert.strictEqual(c1.stderr.toString().split('\n')[0],
'Error: before(): asyncId or triggerId is less than zero ' +
'(asyncId: -1, triggerId: -1)');
assert.strictEqual(c1.status, 1);

const c2 = spawnSync(process.execPath, [__filename, 'test_invalid_trigger_id']);
assert.strictEqual(c2.stderr.toString().split('\n')[0],
'Error: before(): asyncId or triggerId is less than zero ' +
'(asyncId: 1, triggerId: -1)');
assert.strictEqual(c2.status, 1);

const expectedId = async_hooks.newUid();
const expectedTriggerId = async_hooks.newUid();
const expectedType = 'test_emit_before_after_type';

// Verify that if there is no registered hook, then nothing will happen.
async_hooks.emitBefore(expectedId);
async_hooks.emitAfter(expectedId);

initHooks({
onbefore: common.mustCall((id) => assert.strictEqual(id, expectedId)),
onafter: common.mustCall((id) => assert.strictEqual(id, expectedId)),
allowNoInit: true
}).enable();

async_hooks.emitInit(expectedId, expectedType, expectedTriggerId);
async_hooks.emitBefore(expectedId);
async_hooks.emitAfter(expectedId);
49 changes: 49 additions & 0 deletions test/async-hooks/test-emit-init.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const async_hooks = require('async_hooks');
const initHooks = require('./init-hooks');

// Verify that if there is no registered hook, then those invalid parameters
// won't be checked.
assert.doesNotThrow(() => async_hooks.emitInit());

const expectedId = async_hooks.newUid();
const expectedTriggerId = async_hooks.newUid();
const expectedType = 'test_emit_init_type';
const expectedResource = { key: 'test_emit_init_resource' };

const hooks1 = initHooks({
oninit: common.mustCall((id, type, triggerId, resource) => {
assert.strictEqual(id, expectedId);
assert.strictEqual(type, expectedType);
assert.strictEqual(triggerId, expectedTriggerId);
assert.strictEqual(resource.key, expectedResource.key);
})
});

hooks1.enable();

assert.throws(() => async_hooks.emitInit(),
/^RangeError: asyncId must be an unsigned integer$/);
assert.throws(() => async_hooks.emitInit(expectedId),
/^TypeError: type must be a string with length > 0$/);
assert.throws(() => async_hooks.emitInit(expectedId, expectedType, -1),
/^RangeError: triggerId must be an unsigned integer$/);

async_hooks.emitInit(expectedId, expectedType, expectedTriggerId,
expectedResource);

hooks1.disable();

initHooks({
oninit: common.mustCall((id, type, triggerId, resource) => {
assert.strictEqual(id, expectedId);
assert.strictEqual(type, expectedType);
assert.notStrictEqual(triggerId, expectedTriggerId);
assert.strictEqual(resource.key, expectedResource.key);
})
}).enable();

async_hooks.emitInit(expectedId, expectedType, expectedResource);
4 changes: 4 additions & 0 deletions test/async-hooks/test-enable-disable.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ const hook3 = initHooks({ onbefore: onhook3Before, onafter: onhook3After });
// Enabling hook1 and hook3 only, hook2 is still disabled
//
hook1.enable();
// Verify that the hook is enabled even if .enable() is called twice.
hook1.enable();
hook3.enable();

//
Expand All @@ -122,6 +124,8 @@ let disabledHook3 = false;
function onhook2Before() {
if (disabledHook3) return;
hook1.disable();
// Verify that the hook is disabled even if .disable() is called twice.
hook1.disable();
disabledHook3 = true;
}

Expand Down
13 changes: 13 additions & 0 deletions test/parallel/test-async-hooks-run-in-async-id-scope.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const async_hooks = require('async_hooks');

const asyncId = async_hooks.newUid();

assert.notStrictEqual(async_hooks.currentId(), asyncId);

async_hooks.runInAsyncIdScope(asyncId, common.mustCall(() => {
assert.strictEqual(async_hooks.currentId(), asyncId);
}));