Artifact [47ae8c9802]

Artifact 47ae8c9802d5498fb47e7c1bcf0ce259e335dea4488dee3ba28701d8fb665455:

• File cli/repo_clone_internal_test.go — part of check-in [a02c9572da] at 2026-07-23 16:18:24 on branch trunk — clone: add --timeout override and make the deadline reliably fire (#120) (#144)

  The clone command hardcoded a 10-minute context deadline with no way to raise or disable it, and the deadline did not reliably interrupt a slow clone.

  - RepoCloneCmd gains a --timeout flag (default 10m, matching the old behavior; 0 disables the deadline for very large repositories). cloneContext branches away from context.WithTimeout for a non-positive timeout -- WithTimeout(parent, 0) yields an already-expired deadline, which would abort instantly the very clone the flag exists to unbound.

  - The whole-repository crosslink sweep now observes context cancellation. It runs after the exchange-round loop, so it had no round boundary of its own at which the deadline could fire -- on a large repository it could run long past the deadline uninterrupted. manifest.CrosslinkContext checks the context every 256 candidates; manifest.Crosslink stays a background-context wrapper so its ~30 existing callers are unchanged. processResponse gains the same batched check so a large single-round batch is interruptible too, and the sync session's and the serve handler's post-convergence sweeps are wired to their contexts for the same reason.

  Each cancellation point is pinned by a test that fails without it: the crosslink and processResponse tests drive a context that survives the first poll and dies before the second, so only the batched check can observe it; the crosslink-sweep test cancels after a zero-card final round, so nothing before the sweep is left to notice.

  Also adds a clone -> serve -> reclone fidelity regression guard (#114), built with the canonical fossil binary rather than this library's Commit. Commit does not deltify, so a corpus built through it holds zero delta rows and never reaches the serve side's deltified-row expansion branch -- a green result from such a corpus proves nothing about #114. The guard now asserts the source holds delta rows before measuring anything, and compares the second-generation clone's tip, artifact count, blob-store size, delta-row count and checked-out tree content against the source.

  Measured against that corpus (71 artifacts, 54 delta rows), #114 does not reproduce: gen1 and gen2 both hold the same 54 delta rows, the same tip and artifact count, a 1.009 store ratio, identical checked-out content, and pass fossil test-integrity. The receive path re-deltifies what the serve side expanded, via content.Deltify on the crosslink mlink path, so the expansion the serve side performs does not survive into the store. (user: [email protected], size: 2665) [annotate] [blame] [check-ins using]

package cli

import (
	"context"
	"errors"
	"testing"
	"time"
)

// TestCloneContextZeroTimeoutDisablesDeadline pins the branch the --timeout 0
// flag exists for. The naive spelling -- always context.WithTimeout(parent,
// timeout) -- turns 0 into a deadline that has *already* passed, so the clone it
// was meant to unbound would abort instantly instead. Asserting the flag parses
// to 0 cannot catch that; only checking the context it produces can.
func TestCloneContextZeroTimeoutDisablesDeadline(t *testing.T) {
	ctx, cancel := cloneContext(context.Background(), 0)
	defer cancel()

	if deadline, ok := ctx.Deadline(); ok {
		t.Fatalf("timeout 0 produced a context with deadline %v, want none", deadline)
	}

	// Long enough that a zero-duration WithTimeout would certainly have fired.
	time.Sleep(50 * time.Millisecond)

	select {
	case <-ctx.Done():
		t.Fatalf("timeout 0 context is already done (%v); the deadline was not disabled", ctx.Err())
	default:
	}
}

// TestCloneContextNegativeTimeoutDisablesDeadline covers the other side of the
// "not positive" guard: a negative duration is treated as disabled rather than
// as an instantly-expired deadline.
func TestCloneContextNegativeTimeoutDisablesDeadline(t *testing.T) {
	ctx, cancel := cloneContext(context.Background(), -time.Second)
	defer cancel()

	if _, ok := ctx.Deadline(); ok {
		t.Fatal("negative timeout produced a deadline, want none")
	}
	if err := ctx.Err(); err != nil {
		t.Fatalf("negative timeout context is already done: %v", err)
	}
}

// TestCloneContextPositiveTimeoutSetsDeadline pins the ordinary case: a positive
// timeout really does bound the clone, and it fires.
func TestCloneContextPositiveTimeoutSetsDeadline(t *testing.T) {
	ctx, cancel := cloneContext(context.Background(), 30*time.Millisecond)
	defer cancel()

	if _, ok := ctx.Deadline(); !ok {
		t.Fatal("positive timeout produced no deadline")
	}

	select {
	case <-ctx.Done():
		if !errors.Is(ctx.Err(), context.DeadlineExceeded) {
			t.Errorf("ctx.Err() = %v, want context.DeadlineExceeded", ctx.Err())
		}
	case <-time.After(5 * time.Second):
		t.Fatal("positive timeout never fired")
	}
}

// TestCloneContextCancelReleasesContext keeps the returned cancel meaningful in
// the disabled case, so callers get uniform cleanup either way.
func TestCloneContextCancelReleasesContext(t *testing.T) {
	ctx, cancel := cloneContext(context.Background(), 0)
	cancel()

	select {
	case <-ctx.Done():
		if !errors.Is(ctx.Err(), context.Canceled) {
			t.Errorf("ctx.Err() = %v, want context.Canceled", ctx.Err())
		}
	case <-time.After(time.Second):
		t.Fatal("cancel did not release the timeout-disabled context")
	}
}