Skip to content

doc: add guides on writing tests involving promises #20988

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

Closed
joyeecheung wants to merge 3 commits into from
Closed

doc: add guides on writing tests involving promises #20988

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
61354c9
doc: add guides on writing tests involving promises
joyeecheung May 27, 2018
a284df4
fixup! doc: add guides on writing tests involving promises
joyeecheung May 28, 2018
59fd141
fixup! doc: add guides on writing tests involving promises
joyeecheung May 28, 2018
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
Prev Previous commit
Next Next commit
fixup! doc: add guides on writing tests involving promises
  • Loading branch information
@joyeecheung
joyeecheung committed May 28, 2018
commit a284df4805ae0ec38871fc1b4c57656d0a64e5fc
9 changes: 4 additions & 5 deletions doc/guides/writing-tests.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ countdown.dec(); // The countdown callback will be invoked now.

When writing tests involving promises, either make sure that the
`onFulfilled` or the `onRejected` handler is wrapped in
`common.mustCall()` or `common.mustNotCall` accordingly, or
`common.mustCall()` or `common.mustNotCall()` accordingly, or
call `common.crashOnUnhandledRejection()` in the top level of the
test to make sure that unhandled rejections would result in a test
failure. For example:
Expand All @@ -238,20 +238,19 @@ const assert = require('assert');
const fs = require('fs').promises;

// Use `common.crashOnUnhandledRejection()` to make sure unhandled rejections
// will fail the test
// will fail the test.
common.crashOnUnhandledRejection();

// Or, wrap the `onRejected` handler in `common.mustNotCall()`
// Or, wrap the `onRejected` handler in `common.mustNotCall()`.
fs.writeFile('test-file', 'test').catch(common.mustNotCall());

// Or, wrap the `onFulfilled` handler in `common.mustCall()`
// Or, wrap the `onFulfilled` handler in `common.mustCall()`.
fs.readFile('test-file').then(
common.mustCall(
(content) => assert.strictEqual(content.toString(), 'test')
));
Copy link
Copy Markdown
Member

@lpinca lpinca May 27, 2018

Choose a reason for hiding this comment

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

It's an example, but I think it makes sense to add catch() to handle the case where the assertion fails.

Copy link
Copy Markdown
Member Author

@joyeecheung joyeecheung May 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@lpinca that is demonstrated in the first example...also the onFullfilled and the onRejected handler are mutually exclusive so common.mustCall should be able to catch that.

Copy link
Copy Markdown
Member

@lpinca lpinca May 28, 2018

Choose a reason for hiding this comment

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

I'm not sure I understand, this

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

const fsPromises = fs.promises;

fs.writeFileSync('test-file', 'foo');
fsPromises.readFile('test-file').then(
  common.mustCall((content) => {
    assert.strictEqual(content.toString(), 'test');
  })
);

should make the test fails, but it doesn't if common.crashOnUnhandledRejection(); is not added.

Copy link
Copy Markdown
Member Author

@joyeecheung joyeecheung May 28, 2018

Choose a reason for hiding this comment

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

@lpinca I see what you mean now, the catch needs to handle possible failures if the onFulfilled handler is not empty. Thanks for catching that!

```

Copy link
Copy Markdown
Contributor

@vsemozhetbyt vsemozhetbyt May 27, 2018

Choose a reason for hiding this comment

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

Extra empty line.


### Flags

Some tests will require running Node.js with specific command line flags set. To
Expand Down