Skip to content

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 calls buildAllowedResponse 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 call PrepareTestRootDir 3 times. So the oldWd is different every time:

    1. oldWd= /current/directory
    2. after exiting PrepareTestRootDir the current dir is now /tmp/test-gitlab-shellXYZ
    3. The next time we call PrepareTestRootDir the oldWd will now be /tmp/test-gitlab-shellXYZ
    4. 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 for TestGitUploadPackSuccess and not for every subtest.

    We could either call successAPI for every subtest, or memoize response := 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: