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

doc: add throw example #52417

Closed
wants to merge 1 commit into from
Closed

doc: add throw example #52417

wants to merge 1 commit into from

Conversation

AugustinMauroy
Copy link
Member

Add example of throw

Fix: #52357

  • i had run make lint-js-doc

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. test_runner Issues and PRs related to the test runner subsystem. labels Apr 8, 2024
@AugustinMauroy AugustinMauroy changed the title fix: lint doc: add throw example Apr 8, 2024
@aduh95
Copy link
Contributor

aduh95 commented Apr 8, 2024

I'm not sure this belongs in test.md, it's documenting a node:assert method and therefor would make more sense in assert.md:

## `assert.throws(fn[, error][, message])`

We could add a link to assert.md if there's not one already.

@AugustinMauroy
Copy link
Member Author

I'm not sure this belongs in test.md, it's documenting a node:assert method and therefor would make more sense in assert.md:

the idea is to have one or more examples in test.md which cover as many of the test runner's use cases as possible so that someone new to the subject can get an overview.

@AugustinMauroy
Copy link
Member Author

So @aduh95 you thinks it's better to add link to assert instead of having code snippet ?

Comment on lines +102 to +112

function failFunction() {
throw new Error('This error will be caught');
}

// It's the way to catch expected errors i your test
test('test catch error', (t) => {
assert.throws(() => failFunction(), {
message: 'This error will be caught',
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this one that feels out of place. Instead, what we can do is modify synchronous passing test and asynchronous passing test to include use of assert.throws() and await assert.rejects().
Also, we advice against using error.message to prefer error.code, let's showcase that in our docs.

Suggested change
function failFunction() {
throw new Error('This error will be caught');
}
// It's the way to catch expected errors i your test
test('test catch error', (t) => {
assert.throws(() => failFunction(), {
message: 'This error will be caught',
});
});

Comment on lines +592 to +594
error() {
throw new Error('error');
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This code sample is about mocking methods, I don't think assert.throws has anything to do with it.

Suggested change
error() {
throw new Error('error');
},

Comment on lines +597 to +600
assert.throws(() => number.error(), {
message: 'error',
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert.throws(() => number.error(), {
message: 'error',
});

@AugustinMauroy
Copy link
Member Author

I thinks add ressource on learn section is more appropriate

@AugustinMauroy AugustinMauroy deleted the doc-test branch May 21, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs(test-runner): missing example and explain about how to mock error
3 participants