Test spring cleaning: ensure Sandboxes are terminated, clean out obsolete tests, etc. (#217)

* Always terminate Sandboxes in tests
* Call closeEphemeral from onTestFinished
* Remove irrelevant/unnecessary tests and asserts
* Clean up unneccessary comments
* Align Sandbox var naming in exampels
... and tests, while we're at it.

Author: ehdr

CHANGELOG.md

@@ -6,6 +6,7 @@ Both client libraries are pre-1.0, and they have separate versioning.
 
 - Enabled goroutine leak detection for all tests by default.
 - Fixed a few remaining goroutine leaks.
+- Test clean-ups: ensure we always terminate Sandboxes, close ephemeral objects, etc.
 - Added debug logging to `CloudBucketMount` creation in Go, bringing it in line with the JS SDK.
 - Updated the API for creating `CloudBucketMount`s in JS, using the same `modal.cloudBucketMounts.create()` pattern as other Modal objects, bringing it in line with the Go SDK.
 

modal-go/app_test.go

@@ -10,12 +10,10 @@ func TestParseGPUConfig(t *testing.T) {
 	t.Parallel()
 	g := gomega.NewWithT(t)
 
-	// Test empty string returns nil
 	config, err := parseGPUConfig("")
 	g.Expect(err).ShouldNot(gomega.HaveOccurred())
 	g.Expect(config).Should(gomega.BeNil())
 
-	// Test single GPU type
 	config, err = parseGPUConfig("T4")
 	g.Expect(err).ShouldNot(gomega.HaveOccurred())
 	g.Expect(config.GetCount()).To(gomega.Equal(uint32(1)))
@@ -31,7 +29,6 @@ func TestParseGPUConfig(t *testing.T) {
 	g.Expect(config.GetCount()).To(gomega.Equal(uint32(1)))
 	g.Expect(config.GetGpuType()).To(gomega.Equal("A100-80GB"))
 
-	// Test GPU type with count
 	config, err = parseGPUConfig("A100-80GB:3")
 	g.Expect(err).ShouldNot(gomega.HaveOccurred())
 	g.Expect(config.GetCount()).To(gomega.Equal(uint32(3)))
@@ -42,13 +39,11 @@ func TestParseGPUConfig(t *testing.T) {
 	g.Expect(config.GetCount()).To(gomega.Equal(uint32(2)))
 	g.Expect(config.GetGpuType()).To(gomega.Equal("T4"))
 
-	// Test lowercase conversion
 	config, err = parseGPUConfig("a100:4")
 	g.Expect(err).ShouldNot(gomega.HaveOccurred())
 	g.Expect(config.GetCount()).To(gomega.Equal(uint32(4)))
 	g.Expect(config.GetGpuType()).To(gomega.Equal("A100"))
 
-	// Test invalid count formats
 	_, err = parseGPUConfig("T4:invalid")
 	g.Expect(err).Should(gomega.HaveOccurred())
 	g.Expect(err.Error()).To(gomega.ContainSubstring("invalid GPU count: invalid"))

modal-go/client_test.go

@@ -15,7 +15,6 @@ import (
 func TestClientWithLogger(t *testing.T) {
 	g := gomega.NewWithT(t)
 
-	// Use a buffer to capture log output
 	r, w, err := os.Pipe()
 	g.Expect(err).ShouldNot(gomega.HaveOccurred())
 

modal-go/examples/sandbox-poll/main.go

@@ -22,43 +22,43 @@ func main() {
 
 	image := mc.Images.FromRegistry("alpine:3.21", nil)
 
-	// Create a sandbox that waits for input, then exits with code 42
-	sandbox, err := mc.Sandboxes.Create(ctx, app, image, &modal.SandboxCreateParams{
+	// Create a Sandbox that waits for input, then exits with code 42
+	sb, err := mc.Sandboxes.Create(ctx, app, image, &modal.SandboxCreateParams{
 		Command: []string{"sh", "-c", "read line; exit 42"},
 	})
 	if err != nil {
 		log.Fatalf("Failed to create Sandbox: %v", err)
 	}
-	fmt.Printf("Started Sandbox: %s\n", sandbox.SandboxID)
+	fmt.Printf("Started Sandbox: %s\n", sb.SandboxID)
 	defer func() {
-		if err := sandbox.Terminate(context.Background()); err != nil {
-			log.Fatalf("Failed to terminate Sandbox %s: %v", sandbox.SandboxID, err)
+		if err := sb.Terminate(context.Background()); err != nil {
+			log.Fatalf("Failed to terminate Sandbox %s: %v", sb.SandboxID, err)
 		}
 	}()
 
-	initialPoll, err := sandbox.Poll(ctx)
+	initialPoll, err := sb.Poll(ctx)
 	if err != nil {
 		log.Fatalf("Failed to poll Sandbox: %v", err)
 	}
 	fmt.Printf("Poll result while running: %v\n", initialPoll)
 
 	fmt.Println("\nSending input to trigger completion...")
-	_, err = sandbox.Stdin.Write([]byte("hello, goodbye\n"))
+	_, err = sb.Stdin.Write([]byte("hello, goodbye\n"))
 	if err != nil {
 		log.Fatalf("Failed to write to stdin: %v", err)
 	}
-	err = sandbox.Stdin.Close()
+	err = sb.Stdin.Close()
 	if err != nil {
 		log.Fatalf("Failed to close stdin: %v", err)
 	}
 
-	exitCode, err := sandbox.Wait(ctx)
+	exitCode, err := sb.Wait(ctx)
 	if err != nil {
 		log.Fatalf("Failed to wait for Sandbox: %v", err)
 	}
 	fmt.Printf("\nSandbox completed with exit code: %d\n", exitCode)
 
-	finalPoll, err := sandbox.Poll(ctx)
+	finalPoll, err := sb.Poll(ctx)
 	if err != nil {
 		log.Fatalf("Failed to poll Sandbox after completion: %v", err)
 	}

modal-go/examples/sandbox-proxy/main.go

@@ -33,9 +33,9 @@ func main() {
 		Proxy: proxy,
 	})
 	if err != nil {
-		log.Fatalf("Failed to create sandbox: %v", err)
+		log.Fatalf("Failed to create Sandbox: %v", err)
 	}
-	fmt.Printf("Created sandbox with proxy: %s\n", sb.SandboxID)
+	fmt.Printf("Created Sandbox with proxy: %s\n", sb.SandboxID)
 	defer func() {
 		if err := sb.Terminate(context.Background()); err != nil {
 			log.Fatalf("Failed to terminate Sandbox %s: %v", sb.SandboxID, err)

modal-go/examples/sandbox-tunnels/main.go

@@ -26,7 +26,7 @@ func main() {
 	// Create a Sandbox with Python's built-in HTTP server
 	image := mc.Images.FromRegistry("python:3.12-alpine", nil)
 
-	sandbox, err := mc.Sandboxes.Create(ctx, app, image, &modal.SandboxCreateParams{
+	sb, err := mc.Sandboxes.Create(ctx, app, image, &modal.SandboxCreateParams{
 		Command:        []string{"python3", "-m", "http.server", "8000"},
 		EncryptedPorts: []int{8000},
 		Timeout:        1 * time.Minute,
@@ -36,18 +36,18 @@ func main() {
 		log.Fatalf("Failed to create Sandbox: %v", err)
 	}
 	defer func() {
-		if err := sandbox.Terminate(context.Background()); err != nil {
-			log.Fatalf("Failed to terminate Sandbox %s: %v", sandbox.SandboxID, err)
+		if err := sb.Terminate(context.Background()); err != nil {
+			log.Fatalf("Failed to terminate Sandbox %s: %v", sb.SandboxID, err)
 		}
 	}()
 
-	fmt.Printf("Sandbox created: %s\n", sandbox.SandboxID)
+	fmt.Printf("Sandbox created: %s\n", sb.SandboxID)
 
 	fmt.Println("Waiting for server to start...")
 	time.Sleep(3 * time.Second)
 
 	fmt.Println("Getting tunnel information...")
-	tunnels, err := sandbox.Tunnels(ctx, 30*time.Second)
+	tunnels, err := sb.Tunnels(ctx, 30*time.Second)
 	if err != nil {
 		log.Fatalf("Failed to get tunnels: %v", err)
 	}

modal-go/examples/sandbox/main.go

@@ -29,7 +29,7 @@ func main() {
 	if err != nil {
 		log.Fatalf("Failed to create Sandbox: %v", err)
 	}
-	fmt.Printf("sandbox: %s\n", sb.SandboxID)
+	fmt.Printf("Created Sandbox with ID: %s\n", sb.SandboxID)
 	defer func() {
 		if err := sb.Terminate(context.Background()); err != nil {
 			log.Fatalf("Failed to terminate Sandbox %s: %v", sb.SandboxID, err)

modal-go/test/auth_token_manager_test.go

@@ -32,7 +32,6 @@ func (m *mockAuthClient) AuthTokenGet(ctx context.Context, req *pb.AuthTokenGetR
 	}.Build(), nil
 }
 
-// Creates a JWT token for testing
 func createTestJWT(expiry int64) string {
 	token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
 		"exp": expiry,
@@ -53,14 +52,12 @@ func TestAuthTokenManager_DecodeJWT(t *testing.T) {
 	validToken := createTestJWT(123456789)
 	mockClient.setAuthToken(validToken)
 
-	// FetchToken should decode and store the JWT
 	_, err := manager.FetchToken(context.Background())
 	g.Expect(err).ShouldNot(gomega.HaveOccurred())
 
 	g.Expect(manager.GetCurrentToken()).Should(gomega.Equal(validToken))
 }
 
-// Setting the initial token and having it cached.
 func TestAuthTokenManager_InitialFetch(t *testing.T) {
 	t.Parallel()
 	g := gomega.NewWithT(t)
@@ -89,16 +86,13 @@ func TestAuthTokenManager_IsExpired(t *testing.T) {
 
 	manager := modal.NewAuthTokenManager(nil, slog.Default())
 
-	// Test not expired
 	manager.SetToken("token", time.Now().Unix()+3600)
 	g.Expect(manager.IsExpired()).Should(gomega.BeFalse())
 
-	// Test expired
 	manager.SetToken("token", time.Now().Unix()-3600)
 	g.Expect(manager.IsExpired()).Should(gomega.BeTrue())
 }
 
-// Refreshing an expired token. Unlikely to occur since we refresh in background before expiry.
 func TestAuthTokenManager_RefreshExpiredToken(t *testing.T) {
 	t.Parallel()
 	g := gomega.NewWithT(t)
@@ -113,12 +107,10 @@ func TestAuthTokenManager_RefreshExpiredToken(t *testing.T) {
 	manager.SetToken(expiringToken, now-60)
 	mockClient.setAuthToken(freshToken)
 
-	// Start the background refresh goroutine
 	err := manager.Start(context.Background())
 	g.Expect(err).ToNot(gomega.HaveOccurred())
 	defer manager.Stop()
 
-	// Wait for background goroutine to refresh the token
 	g.Eventually(func() string {
 		return manager.GetCurrentToken()
 	}, "1s", "10ms").Should(gomega.Equal(freshToken))
@@ -138,18 +130,15 @@ func TestAuthTokenManager_RefreshNearExpiryToken(t *testing.T) {
 	manager.SetToken(expiringToken, now+60)
 	mockClient.setAuthToken(freshToken)
 
-	// Start the background refresh goroutine
 	err := manager.Start(context.Background())
 	g.Expect(err).ToNot(gomega.HaveOccurred())
 	defer manager.Stop()
 
-	// Wait for background goroutine to refresh the token
 	g.Eventually(func() string {
 		return manager.GetCurrentToken()
 	}, "1s", "10ms").Should(gomega.Equal(freshToken))
 }
 
-// Should error out if no valid token is available.
 func TestAuthTokenManager_GetToken_ExpiredToken(t *testing.T) {
 	t.Parallel()
 	g := gomega.NewWithT(t)

modal-go/test/cls_test.go

@@ -20,7 +20,6 @@ func TestClsCall(t *testing.T) {
 	instance, err := cls.Instance(ctx, nil)
 	g.Expect(err).ShouldNot(gomega.HaveOccurred())
 
-	// Try accessing a non-existent method
 	_, err = instance.Method("nonexistent")
 	g.Expect(err).Should(gomega.BeAssignableToTypeOf(modal.NotFoundError{}))
 

modal-go/test/function_call_test.go

@@ -50,11 +50,9 @@ func TestFunctionSpawn(t *testing.T) {
 	_, err = functionCall.Get(ctx, nil)
 	g.Expect(err).Should(gomega.HaveOccurred())
 
-	// Spawn function with long running input.
 	functionCall, err = sleep.Spawn(ctx, nil, map[string]any{"t": 5})
 	g.Expect(err).ShouldNot(gomega.HaveOccurred())
 
-	// Get is now expected to timeout.
 	timeout := 1 * time.Second
 	_, err = functionCall.Get(ctx, &modal.FunctionCallGetParams{Timeout: &timeout})
 	g.Expect(err).Should(gomega.HaveOccurred())