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

Fix coverage report and add tests #3

Merged
merged 13 commits into from
Mar 30, 2016
Merged

Fix coverage report and add tests #3

merged 13 commits into from
Mar 30, 2016

Conversation

groyoh
Copy link
Member

@groyoh groyoh commented Mar 28, 2016

Fix coverall test report by removing the token from the .travis.yml file. 3 tests were also added. I'll provide a few more tests in upcoming PRs.

st.Expect(t, err, nil)
st.Expect(t, str, bodyBytes)

body = ioutil.NopCloser(&errorReader{})
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if the error handling test should be splitted in a different test. @h2non what's the best practice in Go?

Copy link
Member

Choose a reason for hiding this comment

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

It's better isolated,

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, I'll change it then. Also do you have preference for multi-commits vs squashed commits in PRs?

Copy link
Member

Choose a reason for hiding this comment

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

No particular preferences on that, but for me doing more commits should not be punished.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 943eb2b on groyoh:add_tests into * on vinci-proxy:master*.

@groyoh
Copy link
Member Author

groyoh commented Mar 29, 2016

@h2non I updated the tests to isolate the error cases.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8ee6747 on groyoh:add_tests into * on vinci-proxy:master*.

Yohan Robert added 3 commits March 29, 2016 20:10
The test checks that when ReadBytes fails its error is returned.
The test checks that when Decode fails its error is returned.
@groyoh
Copy link
Member Author

groyoh commented Mar 29, 2016

I added a few more tests in the meantime. When I'm done with the tests, I'll start working on #4.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 7fda371 on groyoh:add_tests into * on vinci-proxy:master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 39c4bc9 on groyoh:add_tests into * on vinci-proxy:master*.

@h2non h2non merged commit fc07435 into vinxi:master Mar 30, 2016
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.

3 participants