From 4d38424e6cbb32d07c74d3b4d760af7afb35742d Mon Sep 17 00:00:00 2001
From: Dan McArdle <d@nmcardle.com>
Date: Sat, 8 Mar 2025 14:49:52 -0500
Subject: [PATCH] cmd/gitannex: Prevent tests from hanging when assertion fails

This fixes another way that the gitannex tests can hang.

The issue is that our test harness explicitly called `wg.Done()` at the
end of each test case, but when assertions checked with [require] fail,
they halt test execution and prevent `wg.Done()` from happening.

A second issue is that we were incorrectly calling [require] functions
in the goroutine that runs the gitannex server. I found that [require]
calls [testing.T.FailNow] under the hood, which says "FailNow must be
called from the goroutine running the test or benchmark function, not
from other goroutines created during the test." [1]

This commit fixes both issues by replacing the explicit synchronization
with a `chan error`. This enables us to run the gitannex server in a
goroutine, interact with the server in the test's goroutine, and then at
then end use [require] on the test-associated goroutine to ensure the
server's error/nil value matches expectations.

[1]: https://pkg.go.dev/testing#T.FailNow
---
 cmd/gitannex/gitannex_test.go | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/cmd/gitannex/gitannex_test.go b/cmd/gitannex/gitannex_test.go
index 7e1ad4cdb..889ca7dbf 100644
--- a/cmd/gitannex/gitannex_test.go
+++ b/cmd/gitannex/gitannex_test.go
@@ -10,7 +10,6 @@ import (
 	"regexp"
 	"runtime"
 	"strings"
-	"sync"
 	"testing"
 	"time"
 
@@ -1393,23 +1392,27 @@ func TestGitAnnexFstestBackendCases(t *testing.T) {
 			handle.remoteName = remoteName
 			handle.remotePrefix = remotePath
 
-			var wg sync.WaitGroup
-			wg.Add(1)
+			serverErrorChan := make(chan error)
 
 			go func() {
-				err := handle.server.run()
-
-				if testCase.expectedError == "" {
-					require.NoError(t, err)
-				} else {
-					require.ErrorContains(t, err, testCase.expectedError)
-				}
-
-				wg.Done()
+				// Run the gitannex server and send the result back to the
+				// goroutine associated with `t`. We can't use `require` here
+				// because it could call `t.FailNow()`, which says it must be
+				// called on the goroutine associated with the test.
+				serverErrorChan <- handle.server.run()
 			}()
-			defer wg.Wait()
 
 			testCase.testProtocolFunc(t, &handle)
+
+			serverError, ok := <-serverErrorChan
+			require.True(t, ok, "Should receive one error/nil from server")
+			require.Empty(t, serverErrorChan)
+
+			if testCase.expectedError == "" {
+				require.NoError(t, serverError)
+			} else {
+				require.ErrorContains(t, serverError, testCase.expectedError)
+			}
 		})
 	}
 }