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

allow to define a custom default response encoder #11

Merged
merged 10 commits into from
Oct 9, 2018

Conversation

b-2-83
Copy link
Contributor

@b-2-83 b-2-83 commented Oct 9, 2018

The default go-kit JSON encoder could be override in the configuration of kitty HTTP transport, so we don't pass the same encoder at each endpoint, but still be able to override it from the endpoint as option.

@coveralls
Copy link

coveralls commented Oct 9, 2018

Pull Request Test Coverage Report for Build 81

  • 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+1.6%) to 78.933%

Files with Coverage Reduction New Missed Lines %
server.go 2 80.0%
Totals Coverage Status
Change from base Build 57: 1.6%
Covered Lines: 296
Relevant Lines: 375

💛 - Coveralls

config.go Outdated Show resolved Hide resolved
http.go Outdated Show resolved Hide resolved
http_test.go Outdated
},
Body: ioutil.NopCloser(bytes.NewBuffer([]byte("OK:override"))),
})
if !overrideCalled {
Copy link
Contributor

@jfbus jfbus Oct 9, 2018

Choose a reason for hiding this comment

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

If override was called by the previous test but not this one, this does not fail...

Copy link
Contributor

@jfbus jfbus Oct 9, 2018

Choose a reason for hiding this comment

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

As long as you test the response body, is it really necessary to test overrideCalled ?

http_test.go Outdated Show resolved Hide resolved
http_test.go Outdated Show resolved Hide resolved
http_test.go Outdated
URL: &url.URL{
Path: "/test/default",
},
Body: ioutil.NopCloser(bytes.NewBuffer([]byte(`{}`))),
Copy link
Contributor

Choose a reason for hiding this comment

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

no body on GET requests => remove this line

http_test.go Outdated
if rec.Code != 200 {
t.Errorf("default HTTP response status expected: %d", rec.Code)
}
body := string(rec.Body.Bytes())
Copy link
Contributor

Choose a reason for hiding this comment

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

body := rec.Body.String()

http_test.go Outdated
"github.com/go-kit/kit/endpoint"
)

type Response struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it used ?

@jfbus jfbus merged commit ecb7832 into master Oct 9, 2018
@jfbus jfbus deleted the fix/option_to_change_default_encoder_decoder branch October 9, 2018 19:17
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

3 participants