mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-12 22:10:25 +00:00
fix NewMockWebServer(): Headers never reached the http client (#11007)
Found while working on https://codeberg.org/forgejo/forgejo/pulls/10798#issuecomment-10083846: The symptom was that the go-github client never returned a `resp.After`, so I tracked down the root cause, which was that, with the mocked http server ... Mocked headers never reached the calling client, because w.WriteHeader() was called before the headers were set in the response. Fix by moving w.WriteHeader() to the right place just before w.Write(), which writes the body. Test added which fails without the fix and succeeds with it. ### Tests - I added test coverage for Go changes... - [X] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [X] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [X] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11007 Reviewed-by: Michael Kriese <michael.kriese@gmx.de> Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org> Co-authored-by: Nils Goroll <nils.goroll@uplex.de> Co-committed-by: Nils Goroll <nils.goroll@uplex.de>
This commit is contained in:
parent
c52ecd2258
commit
2d55ad5585
3 changed files with 34 additions and 3 deletions
|
|
@ -91,8 +91,6 @@ func NewMockWebServer(t *testing.T, liveServerBaseURL, testDataDir string, liveM
|
|||
fixture, err := os.ReadFile(fixturePath)
|
||||
require.NoError(t, err, "missing mock HTTP response: "+fixturePath)
|
||||
|
||||
w.WriteHeader(http.StatusOK)
|
||||
|
||||
// replace any mention of the live HTTP service by the mocked host
|
||||
stringFixture := strings.ReplaceAll(string(fixture), liveServerBaseURL, mockServerBaseURL)
|
||||
if isGh {
|
||||
|
|
@ -104,10 +102,16 @@ func NewMockWebServer(t *testing.T, liveServerBaseURL, testDataDir string, liveM
|
|||
for idx, line := range lines {
|
||||
colonIndex := strings.Index(line, ": ")
|
||||
if colonIndex != -1 {
|
||||
// Because we modified the body with ReplaceAll() above, we need to
|
||||
// remove Content-Length. w.Write() should add it back.
|
||||
header := line[0:colonIndex]
|
||||
if !strings.EqualFold(header, "Content-Length") {
|
||||
w.Header().Set(line[0:colonIndex], line[colonIndex+2:])
|
||||
}
|
||||
} else {
|
||||
// we reached the end of the headers (empty line), so what follows is the body
|
||||
responseBody := strings.Join(lines[idx+1:], "\n")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, err := w.Write([]byte(responseBody))
|
||||
require.NoError(t, err, "writing the body of the HTTP response failed")
|
||||
break
|
||||
|
|
|
|||
24
models/unittest/mock_http_test.go
Normal file
24
models/unittest/mock_http_test.go
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
// Copyright 2026 The Forgejo Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package unittest
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// NOTE: This is a test of the unittest helper itself
|
||||
func TestMockWebServer(t *testing.T) {
|
||||
server := NewMockWebServer(t, "https://example.com", "testdata", false)
|
||||
defer server.Close()
|
||||
request, err := http.NewRequest("GET", server.URL+"/", nil)
|
||||
require.NoError(t, err)
|
||||
response, err := server.Client().Do(request)
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, response.Header["Header"], 1)
|
||||
assert.Equal(t, "value", response.Header["Header"][0])
|
||||
}
|
||||
3
models/unittest/testdata/GET_%2F
vendored
Normal file
3
models/unittest/testdata/GET_%2F
vendored
Normal file
|
|
@ -0,0 +1,3 @@
|
|||
Header: value
|
||||
|
||||
bodydata
|
||||
Loading…
Add table
Add a link
Reference in a new issue