test: add comprehensive integration tests across modctl

[aftersnow]

Summary

• Add shared mock OCI registry (test/helpers/mockregistry.go) with fault injection (latency, partial response, status override, Nth-request failure, per-path faults) for httptest-based integration testing
• Add ~35 integration tests across 8 dimensions: functional correctness, network errors, resource leaks, concurrency safety, stress, data integrity, graceful shutdown, and idempotency
• Document 4 bugs discovered during analysis: ReadCloser leak (bug: Push leaks ReadCloser from PullBlob on both success and error paths #491), splitReader goroutine leak (bug: splitReader goroutine can block indefinitely if pipe reader is abandoned #492), disableProgress data race (bug: disableProgress global variable has data race (no atomic/mutex protection) #493), auth error retry (bug: retry.Do retries 401/403 auth errors instead of failing immediately #494)
• Known bug tests use reverse assertions (CI stays green, tests fail when bugs are fixed)

New Files (13 files, 2433 lines)

File	Tests	Build Tag
test/helpers/mockregistry.go + _test.go	9 self-tests	default
test/helpers/tracking.go	—	default
pkg/modelfile/modelfile_integration_test.go	2	default
pkg/backend/pull_integration_test.go	11	default
pkg/backend/push_integration_test.go	8	default
internal/pb/pb_integration_test.go	2	default
cmd/modelfile/generate_integration_test.go	3	default
cmd/cli_integration_test.go	3	default
pkg/backend/pull_slowtest_test.go	4	slowtest
pkg/backend/push_slowtest_test.go	1	slowtest
pkg/modelfile/modelfile_stress_test.go	2	stress
pkg/backend/pull_stress_test.go	2	stress

Test Plan

• go test ./... — all default tests pass
• go test ./pkg/backend/ -tags slowtest -timeout 300s — retry-dependent tests
• go test ./pkg/modelfile/ ./pkg/backend/ -tags stress -timeout 300s — stress tests
• go test ./internal/pb/ -race — confirms bug: disableProgress global variable has data race (no atomic/mutex protection) #493 data race exists

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive integration testing suite for modctl, covering functional correctness, network resilience, concurrency, and stress scenarios. It includes a new mock OCI registry helper and resource tracking utilities. Review feedback identifies a logic error in a 'KnownBug' test that prevents it from failing when fixed, suggests renaming a misleadingly titled test, recommends optimizing the mock registry's performance by reducing mutex contention during I/O, and advises removing a redundant implementation plan file that duplicates the source code.

[gemini-code-assist]

The if blobTracker.WasClosed() conditional block in TestKnownBug_Push_ReadCloserNotClosed_ErrorPath prevents the test from failing when the bug is fixed. This contradicts the PR's stated strategy for TestKnownBug_ tests, which should use reverse assertions that fail upon a fix to signal that the workaround or documentation is no longer needed. Please remove the conditional and use blobTracker.AssertNotClosed(t) directly.

	blobTracker.AssertNotClosed(t)

[gemini-code-assist]

The test name TestIntegration_Pull_TruncatedBlob is misleading because the implementation (lines 312-317) and comments (lines 307-311) describe a scenario where the full response is delivered but with incorrect bytes (corruption). Truncation (partial response) is already tested in TestIntegration_Pull_PartialResponse. Consider renaming this test to TestIntegration_Pull_CorruptedContentSameLength to better reflect its purpose.

Suggested change

	func TestIntegration_Pull_TruncatedBlob(t *testing.T) {
	func TestIntegration_Pull_CorruptedContentSameLength(t *testing.T) {

[gemini-code-assist]

Holding the global mutex r.mu while performing network I/O (buf.ReadFrom(req.Body)) serializes all registry operations. This creates a bottleneck that can interfere with the effectiveness of concurrent stress tests. It is better to read the request body into a local buffer before acquiring the lock to update the shared state, similar to the pattern used in handleManifest (lines 319-329).

[gemini-code-assist]

This plan file is extremely large and redundant as it duplicates the entire source code of the PR. Additionally, the code snippets within this plan are outdated and contain bugs (e.g., the mockregistry.go implementation here ignores PATCH data) that are fixed in the actual implementation files. This redundancy increases the maintenance burden and can mislead future developers. Consider removing the code blocks or the entire plan file now that the implementation is complete.