clean up health check tests #38
Conversation
|
I may have changed the health check endpoint on this one. going to look into why the tests are still passing |
health/service.go
Outdated
| @@ -95,7 +90,7 @@ func m3ctlHealthCheck(w http.ResponseWriter, r *http.Request) { | |||
| // RegisterHandlers registers health handlers. | |||
| func (s *service) RegisterHandlers(router *mux.Router) { | |||
| log := s.iOpts.Logger() | |||
| router.HandleFunc("", m3ctlHealthCheck) | |||
| router.HandleFunc(s.URLPrefix(), m3ctlHealthCheck) | |||
There was a problem hiding this comment.
I am a little confused as to why the health check was at the root, and how this worked, to begin with.
jskelcy
force-pushed
the
js/config-r2ctrl-cover
branch
from
edd02e4
to
2fd77bb
Compare
|
Fixed the health check issue. I didnt realize it was using a prefixed router. |
1 similar comment
health/service_test.go
Outdated
| @@ -42,18 +45,18 @@ func TestHealthCheck(t *testing.T) { | |||
| rr := httptest.NewRecorder() | |||
| // Create a request to pass to our handler. We don't have any query parameters for now, so we'll | |||
| // pass 'nil' as the third parameter. | |||
| req := http.Request{Method: "GET", RequestURI: "/health"} | |||
| req, err := http.NewRequest("GET", "/health", bytes.NewBuffer(nil)) | |||
There was a problem hiding this comment.
You can just pass nil as third arg here, no need to create a new buffer that never get's used.
There was a problem hiding this comment.
Nice. I figured that would error down the line somewhere but it looks like that totally works.
| h.ResponseTime = time.Since(start) | ||
|
|
||
| h.Status = status |
There was a problem hiding this comment.
There is a slight change to the functionality here where if there is a !ok status we return a 200 with the status as part of the body. Personally, I think this is helpful because you can convey information like being in a bootstrapping state, where the server is up but not ready to receive requests. It is also the case that we hardcode the status to be ok which means that under no circumstances can we return a 500 here anyway.
jskelcy
force-pushed
the
js/config-r2ctrl-cover
branch
from
2fd77bb
to
2df1160
Compare
No description provided.