From c261e6807ca893f8354f6ba34e9f3bc4817b522d Mon Sep 17 00:00:00 2001 From: Marcin Czenko Date: Thu, 23 Oct 2025 05:07:16 +0200 Subject: [PATCH] fixes unprotected write to a map in test, adding race condition detection to CI --- .github/workflows/ci.yml | 2 +- communities/codex_archive_downloader_test.go | 31 ++++++++++++++++---- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a644ec2..4e1ca8c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,7 +46,7 @@ jobs: run: go build -v ./... - name: Run unit tests - run: gotestsum --packages="./communities" -f standard-verbose --rerun-fails -- -v -count=1 + run: gotestsum --packages="./communities" -f standard-verbose --rerun-fails -- -race -v -count=1 - name: Check test coverage for communities package run: | diff --git a/communities/codex_archive_downloader_test.go b/communities/codex_archive_downloader_test.go index 1574c3c..72c2a00 100644 --- a/communities/codex_archive_downloader_test.go +++ b/communities/codex_archive_downloader_test.go @@ -5,6 +5,7 @@ package communities_test import ( "context" + "sync" "testing" "time" @@ -162,14 +163,20 @@ func (suite *CodexArchiveDownloaderTestifySuite) TestMultipleArchives() { // Track the order in which archives are started (deterministic) var startOrder []string + var startOrderMu sync.Mutex downloader.SetOnStartingArchiveDownload(func(hash string, from, to uint64) { + startOrderMu.Lock() startOrder = append(startOrder, hash) + startOrderMu.Unlock() }) // Track completed archives (non-deterministic due to concurrency) completedArchives := make(map[string]bool) + var completedArchivesMu sync.Mutex downloader.SetOnArchiveDownloaded(func(hash string, from, to uint64) { + completedArchivesMu.Lock() completedArchives[hash] = true + completedArchivesMu.Unlock() }) // Initial state verification @@ -189,16 +196,28 @@ func (suite *CodexArchiveDownloaderTestifySuite) TestMultipleArchives() { assert.True(suite.T(), downloader.IsDownloadComplete(), "Download should be complete") assert.Equal(suite.T(), 3, downloader.GetTotalDownloadedArchivesCount(), "Should have downloaded all 3 archives") - // Verify all archives were processed - assert.Len(suite.T(), completedArchives, 3, "Should have completed exactly 3 archives") - assert.Contains(suite.T(), completedArchives, "archive-1", "Should have completed archive-1") - assert.Contains(suite.T(), completedArchives, "archive-2", "Should have completed archive-2") - assert.Contains(suite.T(), completedArchives, "archive-3", "Should have completed archive-3") + // Verify all archives were processed (with proper synchronization) + completedArchivesMu.Lock() + completedLen := len(completedArchives) + hasArchive1 := completedArchives["archive-1"] + hasArchive2 := completedArchives["archive-2"] + hasArchive3 := completedArchives["archive-3"] + completedArchivesMu.Unlock() + + assert.Equal(suite.T(), 3, completedLen, "Should have completed exactly 3 archives") + assert.True(suite.T(), hasArchive1, "Should have completed archive-1") + assert.True(suite.T(), hasArchive2, "Should have completed archive-2") + assert.True(suite.T(), hasArchive3, "Should have completed archive-3") // Verify sorting: archives should be started in most-recent-first order (deterministic) // This tests the internal sorting logic before concurrency begins + startOrderMu.Lock() + startOrderCopy := make([]string, len(startOrder)) + copy(startOrderCopy, startOrder) + startOrderMu.Unlock() + expectedStartOrder := []string{"archive-3", "archive-2", "archive-1"} - assert.Equal(suite.T(), expectedStartOrder, startOrder, "Archives should be started in most-recent-first order") + assert.Equal(suite.T(), expectedStartOrder, startOrderCopy, "Archives should be started in most-recent-first order") suite.T().Log("✅ Multiple archives test passed") suite.T().Logf(" - Completed %d out of %d archives", len(completedArchives), 3)