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: test async-hook triggerId invariant #13328

Closed
wants to merge 1 commit into from
Closed

test: test async-hook triggerId invariant #13328

wants to merge 1 commit into from

Conversation

dszakallas
Copy link
Contributor

@dszakallas dszakallas commented May 31, 2017

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Adds two tests to check that the triggerId invariant is satisfied, that is async resources with the same cause have the same triggerId, while those with different cause have different triggerIds.

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label May 31, 2017
@mscdex mscdex added the async_hooks Issues and PRs related to the async hooks subsystem. label May 31, 2017
process.nextTick(() => {
process.nextTick(() => {
triggerId1 = triggerId();
assert.notEqual(triggerId0, triggerId1, "Async resources having different causal ancestry should have different triggerIds")
Copy link
Member

Choose a reason for hiding this comment

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

long line here. please line break at <= 80 chars and run make lint :-)

Copy link
Member

Choose a reason for hiding this comment

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

Also, this should likely use assert.notStrictEqual()

})
process.nextTick(() => {
const triggerId2 = triggerId();
assert.equal(triggerId1, triggerId2,
Copy link
Member

Choose a reason for hiding this comment

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

assert.strictEqual() here

@trevnorris
Copy link
Contributor

Mind adding a comment in the commit message body about what important scenario/etc. this is testing for?

Add tests for checking the behavior of async_hooks.triggerId.
It should return different ids when called in callbacks having
different ancestry paths.
It should return the same id when called in callbacks having
the same ancestry path.
@dszakallas
Copy link
Contributor Author

updated!

Copy link
Contributor

@trevnorris trevnorris left a comment

Choose a reason for hiding this comment

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

Thanks for editing the commit message.

Copy link
Member

@AndreasMadsen AndreasMadsen left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks.

@AndreasMadsen
Copy link
Member

@addaleax
Copy link
Member

addaleax commented Jun 3, 2017

Landed in fcab69c, thank you for the PR!

@addaleax addaleax closed this Jun 3, 2017
addaleax pushed a commit that referenced this pull request Jun 3, 2017
Add tests for checking the behavior of async_hooks.triggerId.
It should return different ids when called in callbacks having
different ancestry paths.
It should return the same id when called in callbacks having
the same ancestry path.

PR-URL: #13328
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
addaleax pushed a commit that referenced this pull request Jun 3, 2017
Add tests for checking the behavior of async_hooks.triggerId.
It should return different ids when called in callbacks having
different ancestry paths.
It should return the same id when called in callbacks having
the same ancestry path.

PR-URL: #13328
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
jasnell pushed a commit that referenced this pull request Jun 5, 2017
Add tests for checking the behavior of async_hooks.triggerId.
It should return different ids when called in callbacks having
different ancestry paths.
It should return the same id when called in callbacks having
the same ancestry path.

PR-URL: #13328
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
@jasnell jasnell mentioned this pull request Jun 5, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants