Follow-up from "Add golangci-lint to CI job"
The following discussion from !839 (merged) should be addressed:
-
@jaime started a discussion: (+2 comments) @stanhu This is failing because in the acceptance test we use
successAPI
only once (this is key!) which callsbuildAllowedResponse
in https://gitlab.com/gitlab-org/gitlab-shell/-/blob/main/cmd/gitlab-sshd/acceptance_test.go?ref_type=heads#L186 every time we call the mocked endpoint/api/v4/internal/allowed
.For the test
TestGitUploadPackSuccess
that is failing, we have 3 sessions that will callPrepareTestRootDir
3 times. So theoldWd
is different every time:oldWd= /current/directory
- after exiting
PrepareTestRootDir
the current dir is now/tmp/test-gitlab-shellXYZ
- The next time we call
PrepareTestRootDir
theoldWd
will now be/tmp/test-gitlab-shellXYZ
- By the time we reach this cleanup function, we try to cd to
/tmp/test-gitlab-shellXYZ
which has been removed by line 37
The problem is that we only call
t.Cleanup
once forTestGitUploadPackSuccess
and not for every subtest.We could either call
successAPI
for every subtest, or memoizeresponse := buildAllowedResponse
inside that handler.Moving
response
before the handler does the trick:diff --git a/cmd/gitlab-sshd/acceptance_test.go b/cmd/gitlab-sshd/acceptance_test.go index 2319dd3..2cd405d 100644 --- a/cmd/gitlab-sshd/acceptance_test.go +++ b/cmd/gitlab-sshd/acceptance_test.go @@ -164,7 +164,7 @@ type customHandler struct { func successAPI(t *testing.T, handlers ...customHandler) http.Handler { t.Helper() - + response := buildAllowedResponse(t, "responses/allowed_without_console_messages.json") return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { t.Logf("gitlab-api-mock: received request: %s %s", r.Method, r.RequestURI) w.Header().Set("Content-Type", "application/json") @@ -183,8 +183,6 @@ func successAPI(t *testing.T, handlers ...customHandler) http.Handler { case "/api/v4/internal/authorized_keys": fmt.Fprintf(w, `{"id":1, "key":"%s"}`, r.FormValue("key")) case "/api/v4/internal/allowed": - response := buildAllowedResponse(t, "responses/allowed_without_console_messages.json") - _, err := fmt.Fprint(w, response) require.NoError(t, err) default: