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
Next Next commit
doc: add guides on writing tests involving promises 
Mention `common.crashOnUnhandledRejection()` and wrapping the
handlers in `common.mustCall()` or `common.mustNotCall()`
  • Loading branch information
@joyeecheung
joyeecheung committed May 27, 2018
commit 61354c94b0273078170f7804b3df386e0a654553
28 changes: 28 additions & 0 deletions doc/guides/writing-tests.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,34 @@ countdown.dec();
countdown.dec(); // The countdown callback will be invoked now.
```

#### Testing promises

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
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.

common.mustNotCall -> common.mustNotCall()?

call `common.crashOnUnhandledRejection()` in the top level of the
test to make sure that unhandled rejections would result in a test
failure. For example:

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

// Use `common.crashOnUnhandledRejection()` to make sure unhandled rejections
// will fail the test
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.

test -> test.?

common.crashOnUnhandledRejection();

// Or, wrap the `onRejected` handler in `common.mustNotCall()`
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.

Ditto, period)

fs.writeFile('test-file', 'test').catch(common.mustNotCall());

// Or, wrap the `onFulfilled` handler in `common.mustCall()`
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.

Ditto, period)

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

Expand Down