Give each import request in TestMigrator_ImportDouble its own reader (#2220)

The test passed one strings.Reader as the body of both POSTs. client.Do
returns once the response headers arrive, and the import answers 202 before
the transport has finished copying the body, so the second http.NewRequest
reads the reader's Len to set ContentLength while the first request's
writeLoop is still advancing it. The race detector caught it on CI as a write
in strings.(*Reader).WriteTo against a read in NewRequestWithContext, failing
a test nothing had touched.

Reproduced in isolation to confirm the mechanism rather than infer it from the
trace: a handler that answers 202 without draining an 8 MiB body, two requests
sharing one reader, and -race reports strings.(*Reader).Len in
NewRequestWithContext against strings.(*Reader).Read on every run. It does not
reproduce in this package locally, which is why it reads as a flake.

Both requests now build their own reader over the same content. The second one
carries a full body where before it inherited a consumed one, which is closer
to what the case is about: a second import arriving while the first is running
still has to be refused.
This commit is contained in:
Dmitry Verkhoturov
2026-08-23 14:58:31 -05:00
committed by GitHub
parent 6f40926241
commit 389189afcf
+7 -3
View File
@@ -280,10 +280,14 @@ func TestMigrator_ImportDouble(t *testing.T) {
for i := range 50 {
recs = append(recs, fmt.Sprintf(tmpl, i))
}
r := strings.NewReader(`{"version":1}` + strings.Join(recs, "\n")) // reader with 10k records
// each request needs its own reader. client.Do returns once the response headers are in, which
// for an accepted import is before the transport's writeLoop has finished copying the body, so
// handing the same strings.Reader to the second NewRequest races that copy: NewRequest reads
// Len() to set ContentLength while WriteTo is still advancing it
body := `{"version":1}` + strings.Join(recs, "\n")
client := &http.Client{Timeout: waitTimeout}
defer client.CloseIdleConnections()
req, err := http.NewRequest("POST", ts.URL+"/api/v1/admin/import?site=remark42&provider=native", r)
req, err := http.NewRequest("POST", ts.URL+"/api/v1/admin/import?site=remark42&provider=native", strings.NewReader(body))
require.NoError(t, err)
req.SetBasicAuth("admin", "password")
assert.NoError(t, err)
@@ -294,7 +298,7 @@ func TestMigrator_ImportDouble(t *testing.T) {
client = &http.Client{Timeout: 5 * time.Second}
defer client.CloseIdleConnections()
req, err = http.NewRequest("POST", ts.URL+"/api/v1/admin/import?site=remark42&provider=native", r)
req, err = http.NewRequest("POST", ts.URL+"/api/v1/admin/import?site=remark42&provider=native", strings.NewReader(body))
require.NoError(t, err)
req.SetBasicAuth("admin", "password")
assert.NoError(t, err)