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

Add note on testInDebug #4240

Merged
merged 1 commit into from
Mar 18, 2016
Merged
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ taken straight from the Ruby on Rails guide:
* Update the documentation, the surrounding one, examples elsewhere, guides,
whatever is affected by your contribution

Syntax:
## Syntax:

* Two spaces, no tabs.
* No trailing whitespace. Blank lines should not have any space.
Expand All @@ -99,4 +99,10 @@ Syntax:

And in case we didn't emphasize it enough: we love tests!

## Writing Tests

* We do write tests for our warns and assertion messages, using the `assert.expectAssertion()` and `assert.expectWarning()` helpers.
* Because Travis runs tests in the `production` environment, assertions and warnings are stripped out. To avoid tests on warning/assertion messages failing for your PR, use the `testInDebug` helper to skip them in production. See [this](https://github.com/emberjs/data/blob/b3eb9c098ef8c2cf9ff3378ed079769782c02bb5/tests/integration/adapter/queries-test.js#L32) example.

Copy link
Member

Choose a reason for hiding this comment

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

Typo: envrionment -> environment

I think it would be good to add a small sample which clarifies where to import that testInDebug helper from and how it is used instead of test...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll link to an existing test using it

Copy link
Member

Choose a reason for hiding this comment

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

That's good also! Just in case you haven't thought already about that: I would suggest a link using the specific commit, and not master, so the link stays resilient to changes. You can achieve that by pressing y to get the canonical form: https://help.github.com/articles/getting-permanent-links-to-files/.


NOTE: Partially copied from https://raw.github.com/thoughtbot/factory_girl_rails/master/CONTRIBUTING.md