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

Strengthen tests that check for undefined #254

Merged
merged 2 commits into from
Dec 23, 2016

Conversation

alextes
Copy link
Contributor

@alextes alextes commented Dec 22, 2016

Since we're checking for an undefined property, it becomes relevant to
check the rest of the object does look the way we expect. If the returned body object breaks, through parsing problems or otherwise, this test could break but pass.

I ran into this working on a larger PR coming up :3.

  • I don't care for the comment I added. If you feel the reason is clear, I could remove it.
  • If you feel there's a way to address this issue in a cleaner way, I'd happily implement it.
  • If you feel breaking something like parsing but this test still passing is not something these specific tests should care about because of the assumption another test somewhere should break too. I can get behind that, and you can close the PR. I like the idea of fixing tests you'd expect to break but don't, and making them break. On the other hand, you shouldn't test all of the dependencies every test relies on. Your call!

Since we're checking for an undefined property it becomes relevant to
check the rest of the object does look the way we expect. Otherwise this
test could break but pass simply by virtue of the whole object being
broken.
@alextes
Copy link
Contributor Author

alextes commented Dec 22, 2016

I couldn't help it, sorry not sorry.
bitmoji

@floatdrop
Copy link
Contributor

I don't care for the comment I added. If you feel the reason is clear, I could remove it.

@alextes Yeah, tests should be descriptive by themselves. 👍

@alextes
Copy link
Contributor Author

alextes commented Dec 23, 2016

@floatdrop Added two words to the descriptive error message. Removed the comment. I feel this would also work.

@floatdrop floatdrop merged commit 4b7fb5d into sindresorhus:master Dec 23, 2016
@alextes alextes changed the title Strengthen expect undefined tests Strengthen tests that check for undefined Dec 23, 2016
@alextes alextes deleted the strengthen-test branch March 23, 2017 22:27
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

2 participants