Skip to content

Commit

Permalink
fix(registry): fix inaccurate tests covered by catch-all-200 repsonse…
Browse files Browse the repository at this point in the history
… regserver handler

commit d3fbce4 moved the "healthcheck" handler from "/" to "/health", and in the process uncovered a number of tests that were reporting false-positives. by registring a simple function that responds with a JSON payload of "200 - OK", a number of routes that weren't being created by the registry server were falling back to this OK response.

Because our tests are mainly relying on response codes and not the actual payload of a response, this catch-all reponse made it seem like publication was happening, when in fact nothing was occuring at all. not good. I've addressed the failing tests, but let's make a point of _never_ using the "/" route. On top of that, we should be adjusting our code to plumb server response data back to the user, and checking that data in tests.
  • Loading branch information
b5 committed Feb 7, 2020
1 parent d3fbce4 commit 1d30975
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 3 deletions.
7 changes: 6 additions & 1 deletion cmd/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,12 @@ func TestCommandsIntegration(t *testing.T) {
t.Skip(err.Error())
}

_, registryServer := regmock.NewMockServer()
reg, cleanup, err := regmock.NewTempRegistry("registry", "qri_test_commands_integration")
if err != nil {
t.Fatal(err)
}
defer cleanup()
_, registryServer := regmock.NewMockServerRegistry(*reg)

path := filepath.Join(os.TempDir(), "qri_test_commands_integration")
// fmt.Printf("test filepath: %s\n", path)
Expand Down
2 changes: 1 addition & 1 deletion lib/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (r *RemoteMethods) Unpublish(p *PublicationParams, res *reporef.DatasetRef)
// for allowing partial completion where only one of logs or dataset pushing works
// by doing both in parallel and reporting issues on both
if removeLogsErr := r.inst.RemoteClient().RemoveLogs(ctx, repo.ConvertToDsref(ref), addr); removeLogsErr != nil {
log.Error("removing logs: %s", removeLogsErr.Error())
log.Errorf("removing logs: %s", removeLogsErr.Error())
}

if err := r.inst.RemoteClient().RemoveDataset(ctx, ref, addr); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion registry/regserver/handlers/profile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ func TestPostProfiles(t *testing.T) {
}
]`

req, err := http.NewRequest("POST", fmt.Sprintf("%s/profiles", s.URL), strings.NewReader(profiles))
req, err := http.NewRequest("POST", fmt.Sprintf("%s/registry/profiles", s.URL), strings.NewReader(profiles))
if err != nil {
t.Error(err.Error())
return
Expand Down

0 comments on commit 1d30975

Please sign in to comment.