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

Replaced try-catch statements with assertions #1427

Merged
merged 2 commits into from
May 26, 2017

Conversation

fearphage
Copy link
Member

Replaced try-catch statements with (assert|refute).exception() to make them more explicit about their intent. I also added an ESLint rule to disallow try-catch in the tests directory.

@coveralls
Copy link

coveralls commented May 26, 2017

Coverage Status

Coverage remained the same at 94.896% when pulling 9e23c4a on fearphage:remove-try-from-tests into d8dac84 on sinonjs:master.

Copy link
Member

@lucasfcosta lucasfcosta left a comment

Choose a reason for hiding this comment

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

Good job! This LGTM.

I also have had a hard time thinking about how to check errors before. I think that by making the whole codebase uniforme we can make it easier for other contributors to find out the right way to do it.

@fatso83 fatso83 merged commit 3af985a into sinonjs:master May 26, 2017
@fearphage fearphage deleted the remove-try-from-tests branch July 8, 2018 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants