Skip to content

Commit 433e0bb

Browse files
khaliqgantclaude
andcommitted
test(mount): drop synthetic Results injection; assert POST /fs/file == 0
The TestBulkMigrationReducesHTTPCalls proof rewrote the mock /fs/bulk response to include synthetic Results revisions — which the real server does not return — so reconcileBulkWrite skipped its fallback ReadFile calls only in the test. That masked production traffic and the reviewer flagged it correctly (P2 on PR #63). Drop the injection. Track POST vs GET separately on /fs/file: - assert 1 POST /fs/bulk (batching works) - assert 0 POST /fs/file (no per-file writes — the load-bearing guarantee) - log GET /fs/file count (revision read-back, expected until server returns per-file revisions) Test now passes against the real server's response shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f66a10f commit 433e0bb

1 file changed

Lines changed: 30 additions & 44 deletions

File tree

internal/mountsync/syncer_test.go

Lines changed: 30 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -660,11 +660,31 @@ func TestBulkMigrationReducesHTTPCalls(t *testing.T) {
660660
workspaceID := "ws_mount_bulk_http_volume"
661661
handler := newMountsyncAPIHandler(t, store)
662662

663+
// NOTE on what this test proves:
664+
//
665+
// The relayfile server's /fs/bulk response today only carries
666+
// {written, errorCount, errors, correlationId} — it does NOT return
667+
// per-file revisions. That means after a bulk write, the Syncer's
668+
// reconcileBulkWrite() falls back to a per-file GET /fs/file to learn
669+
// the new revision for If-Match on the next cycle. So in production
670+
// the post-bulk traffic looks like: 1 POST /fs/bulk + N GET /fs/file.
671+
//
672+
// The migration's guarantee is: **zero per-file WRITES (POST /fs/file)
673+
// after the first batch.** That is what this test asserts. It counts
674+
// POST and GET separately on /fs/file so GET-based revision read-back
675+
// does not trigger a false failure. An earlier version of this test
676+
// mocked synthetic `Results` into the bulk response, which masked the
677+
// GET fallback and reviewers flagged it as non-representative.
678+
//
679+
// When /fs/bulk grows per-file revision output on the server, the GET
680+
// fallback goes away and this test can tighten to "zero total requests
681+
// on /fs/file" by removing the PostCount filter below.
663682
var requestCounts sync.Map
664683
var requestsMu sync.Mutex
665684
requests := make([]string, 0)
666685
var fsFileRequestsMu sync.Mutex
667686
fsFileRequests := make([]string, 0)
687+
var fsFilePostCount atomic.Int32
668688
counterFor := func(path string) *atomic.Int32 {
669689
counter, _ := requestCounts.LoadOrStore(path, &atomic.Int32{})
670690
return counter.(*atomic.Int32)
@@ -679,53 +699,14 @@ func TestBulkMigrationReducesHTTPCalls(t *testing.T) {
679699
fsFileRequestsMu.Lock()
680700
fsFileRequests = append(fsFileRequests, r.Method+" "+r.URL.String())
681701
fsFileRequestsMu.Unlock()
682-
}
683-
684-
var body []byte
685-
if strings.HasSuffix(r.URL.Path, "/fs/bulk") && r.Method == http.MethodPost {
686-
var err error
687-
body, err = io.ReadAll(r.Body)
688-
if err != nil {
689-
t.Fatalf("read bulk request body failed: %v", err)
702+
if r.Method == http.MethodPost {
703+
fsFilePostCount.Add(1)
690704
}
691-
r.Body = io.NopCloser(bytes.NewReader(body))
692705
}
693706

694707
recorder := httptest.NewRecorder()
695708
handler.ServeHTTP(recorder, r)
696709

697-
if strings.HasSuffix(r.URL.Path, "/fs/bulk") && r.Method == http.MethodPost && recorder.Code == http.StatusAccepted {
698-
var payload struct {
699-
Files []BulkWriteFile `json:"files"`
700-
}
701-
if err := json.Unmarshal(body, &payload); err != nil {
702-
t.Fatalf("unmarshal bulk request body failed: %v", err)
703-
}
704-
705-
var response BulkWriteResponse
706-
if err := json.Unmarshal(recorder.Body.Bytes(), &response); err != nil {
707-
t.Fatalf("unmarshal bulk response failed: %v", err)
708-
}
709-
710-
response.Results = make([]BulkWriteResult, 0, len(payload.Files))
711-
for _, file := range payload.Files {
712-
stored, err := store.ReadFile(workspaceID, normalizeRemotePath(file.Path))
713-
if err != nil {
714-
t.Fatalf("read stored bulk result for %s failed: %v", file.Path, err)
715-
}
716-
response.Results = append(response.Results, BulkWriteResult{
717-
Path: stored.Path,
718-
Revision: stored.Revision,
719-
ContentType: stored.ContentType,
720-
})
721-
}
722-
723-
recorder.Body.Reset()
724-
if err := json.NewEncoder(recorder.Body).Encode(response); err != nil {
725-
t.Fatalf("encode augmented bulk response failed: %v", err)
726-
}
727-
}
728-
729710
for key, values := range recorder.Header() {
730711
for _, value := range values {
731712
w.Header().Add(key, value)
@@ -772,10 +753,15 @@ func TestBulkMigrationReducesHTTPCalls(t *testing.T) {
772753
t.Fatalf("expected exactly one bulk request to %s, got %d", bulkPath, got)
773754
}
774755

775-
filePath := fmt.Sprintf("/v1/workspaces/%s/fs/file", workspaceID)
776-
if got := counterFor(filePath).Load(); got != 0 {
777-
t.Fatalf("expected zero per-file requests to %s, got %d: %v (all requests: %v)", filePath, got, fsFileRequests, requests)
756+
// The migration's load-bearing guarantee: no per-file POSTs (writes)
757+
// to /fs/file. GETs on /fs/file from the post-bulk revision read-back
758+
// are expected until the server returns per-file revisions in the
759+
// bulk response — they're tracked in fsFileRequests for visibility
760+
// but don't fail the test.
761+
if got := fsFilePostCount.Load(); got != 0 {
762+
t.Fatalf("expected zero POST /fs/file requests (per-file writes) after bulk migration, got %d: %v", got, fsFileRequests)
778763
}
764+
t.Logf("bulk migration verified: 1 POST /fs/bulk, 0 POST /fs/file, %d GET /fs/file (revision read-back)", len(fsFileRequests))
779765
}
780766

781767
func TestBulkWrite_ChunkAtThreshold(t *testing.T) {

0 commit comments

Comments
 (0)