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 tests for defaults #118

Merged
merged 3 commits into from Mar 2, 2016
Merged

Conversation

@ThisIsMissEm
Copy link
Contributor

ThisIsMissEm commented Mar 2, 2016

This PR adds tests for checking that Defaults are actually applied correctly; Turns out we weren't quite at 100% Code Coverage, it just appeared like we were.

See also: #117

miksago added 2 commits Mar 2, 2016
Previously they were under "Events", which didn't actually seem correct, given they weren't testing event logic.
This test does currently fail, see #117
@ThisIsMissEm ThisIsMissEm force-pushed the ThisIsMissEm:add-tests-for-defaults branch from 76f5f9a to 59f0244 Mar 2, 2016
@ThisIsMissEm

This comment has been minimized.

Copy link
Contributor Author

ThisIsMissEm commented Mar 2, 2016

Updated to show that options that exist in defaults, but not in request options, are also still present.

@ThisIsMissEm

This comment has been minimized.

Copy link
Contributor Author

ThisIsMissEm commented Mar 2, 2016

If you would like, I can also provide the PR for the fix?

@Marsup

This comment has been minimized.

Copy link
Member

Marsup commented Mar 2, 2016

Seems like a simple enough fix on https://github.com/hapijs/wreck/blob/master/lib/index.js#L69, you should go ahead and fix it in the same PR.

@ThisIsMissEm

This comment has been minimized.

Copy link
Contributor Author

ThisIsMissEm commented Mar 2, 2016

Updated. /cc @Marsup

@geek geek added the bug label Mar 2, 2016
@geek geek added this to the 7.0.1 milestone Mar 2, 2016
@geek geek self-assigned this Mar 2, 2016
geek added a commit that referenced this pull request Mar 2, 2016
Add tests for defaults
@geek geek merged commit 289fd7a into hapijs:master Mar 2, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.