diff --git a/.github/workflows/mmctl-test-template.yml b/.github/workflows/mmctl-test-template.yml index 3e096c67bc..079b23ab18 100644 --- a/.github/workflows/mmctl-test-template.yml +++ b/.github/workflows/mmctl-test-template.yml @@ -56,7 +56,7 @@ jobs: docker compose --ansi never ps - name: Run mmctl Tests env: - BUILD_IMAGE: mattermostdevelopment/mattermost-build-server:${{ steps.go.outputs.GO_VERSION }} + BUILD_IMAGE: mattermost/mattermost-build-server:${{ steps.go.outputs.GO_VERSION }} run: | if [[ ${{ github.ref_name }} == 'master' ]]; then export TESTFLAGS="-timeout 90m -race" @@ -66,6 +66,7 @@ jobs: docker run --net ghactions_mm-test \ --ulimit nofile=8096:8096 \ --env-file=server/build/dotenv/test.env \ + --env TEST_DATABASE_POSTGRESQL_DSN="${{ inputs.datasource }}" \ --env MM_SQLSETTINGS_DATASOURCE="${{ inputs.datasource }}" \ --env MMCTL_TESTFLAGS="$TESTFLAGS" \ -v $(go env GOCACHE):/go/cache \ diff --git a/.github/workflows/server-test-template.yml b/.github/workflows/server-test-template.yml index ac6d747cbe..e79099c35e 100644 --- a/.github/workflows/server-test-template.yml +++ b/.github/workflows/server-test-template.yml @@ -22,6 +22,19 @@ on: required: false type: boolean default: false + # -- Test sharding inputs (leave defaults for non-sharded callers) -- + shard-index: + required: false + type: number + default: -1 # -1 = no sharding; run all tests + shard-total: + required: false + type: number + default: 1 + +permissions: + id-token: write + contents: read jobs: test: @@ -33,6 +46,21 @@ jobs: steps: - name: Checkout mattermost project uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Restore test timing data + if: inputs.shard-total > 1 + id: timing-cache + uses: actions/cache/restore@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3 + with: + path: | + server/prev-report.xml + server/prev-gotestsum.json + # Always restore from master — timing is only saved on the default + # branch and is stable enough for shard balancing. + key: server-test-timing-master + restore-keys: | + server-test-timing- + - name: Calculate Golang Version id: go working-directory: ./server @@ -55,13 +83,83 @@ jobs: cat ../tests/test-data.ldif | docker compose --ansi never exec -T openldap bash -c 'ldapadd -x -D "cn=admin,dc=mm,dc=test,dc=com" -w mostest'; docker compose --ansi never exec -T minio sh -c 'mkdir -p /data/mattermost-test'; docker compose --ansi never ps + + # ── Test-level sharding ──────────────────────────────────────────── + # When shard-total > 1, we split tests across N parallel runners. + # + # Two-tier splitting strategy: + # - "Light" packages (< 5 min): assigned whole to a shard + # - "Heavy" packages (≥ 5 min, e.g. api4, app): individual tests + # are distributed across shards using -run regex filters + # + # See server/scripts/shard-split.js for the full algorithm. + # ───────────────────────────────────────────────────────────────────── + + - name: Setup Go for test discovery + if: inputs.shard-total > 1 + uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0 + with: + go-version: ${{ steps.go.outputs.GO_VERSION }} + + - name: Split tests across shards + if: inputs.shard-total > 1 + id: test_split + working-directory: server + env: + SHARD_INDEX: ${{ inputs.shard-index }} + SHARD_TOTAL: ${{ inputs.shard-total }} + run: | + set -euo pipefail + + # ── List all test packages ── + echo "::group::Listing test packages" + TE_PKGS=$(find ./public/ ./ -name '*_test.go' -not -path './enterprise/*' -not -path './cmd/mmctl/*' 2>/dev/null \ + | sed 's|/[^/]*$||' | sort -u \ + | sed 's|^\./|github.com/mattermost/mattermost/server/v8/|' \ + | sed 's|github.com/mattermost/mattermost/server/v8/public/|github.com/mattermost/mattermost/server/public/|') + EE_PKGS=$(find ./enterprise/ -name '*_test.go' 2>/dev/null \ + | sed 's|/[^/]*$||' | sort -u \ + | sed 's|^\./|github.com/mattermost/mattermost/server/v8/|') + ALL_PKGS=$(printf '%s\n%s' "$TE_PKGS" "$EE_PKGS" | grep -v '^$' | sort -u) + TOTAL_PKGS=$(echo "$ALL_PKGS" | wc -l) + echo "Found $TOTAL_PKGS test packages" + echo "::endgroup::" + + if [[ "$TOTAL_PKGS" -eq 0 ]]; then + echo "WARNING: No test packages found" + echo "has_packages=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + + echo "$ALL_PKGS" > all-packages.txt + + # ── Run shard solver ── + node scripts/shard-split.js + + echo "has_packages=true" >> "$GITHUB_OUTPUT" + + - name: Run Tests env: - BUILD_IMAGE: mattermostdevelopment/mattermost-build-server:${{ steps.go.outputs.GO_VERSION }} + BUILD_IMAGE: mattermost/mattermost-build-server:${{ steps.go.outputs.GO_VERSION }} run: | if [[ ${{ github.ref_name }} == 'master' && ${{ inputs.fullyparallel }} != true ]]; then export RACE_MODE="-race" fi + + TEST_TARGET="test-server${RACE_MODE}" + BUILD_NUMBER="${GITHUB_HEAD_REF}-${GITHUB_RUN_ID}" + DOCKER_CMD="make ${TEST_TARGET}" + + # When sharding is active, use the multi-run wrapper script. + # run-shard-tests.sh detects heavy runs itself and falls back to + # light-only mode when shard-heavy-runs.txt is absent or empty. + if [[ "${{ inputs.shard-total }}" -gt 1 && -f server/shard-te-packages.txt ]]; then + cp server/scripts/run-shard-tests.sh server/run-shard-tests.sh + chmod +x server/run-shard-tests.sh + DOCKER_CMD="/mattermost/server/run-shard-tests.sh" + fi + docker run --net ghactions_mm-test \ --ulimit nofile=8096:8096 \ --env-file=server/build/dotenv/test.env \ @@ -73,10 +171,13 @@ jobs: --env ENABLE_COVERAGE="${{ inputs.enablecoverage }}" \ -v $(go env GOCACHE):/go/cache \ -e GOCACHE=/go/cache \ + --env RACE_MODE \ + --env TEST_TARGET \ + --env BUILD_NUMBER \ -v $PWD:/mattermost \ -w /mattermost/server \ $BUILD_IMAGE \ - make test-server$RACE_MODE BUILD_NUMBER=$GITHUB_HEAD_REF-$GITHUB_RUN_ID + $DOCKER_CMD - name: Upload coverage to Codecov if: ${{ inputs.enablecoverage }} uses: codecov/codecov-action@v5 @@ -99,3 +200,4 @@ jobs: server/cover.out server/test-name server/pr-number + diff --git a/.gitignore b/.gitignore index 473d44b7b3..e0bdd41dc8 100644 --- a/.gitignore +++ b/.gitignore @@ -163,3 +163,6 @@ docker-compose.override.yaml **/CLAUDE.local.md CLAUDE.md .cursorrules +server/prev-report.xml +server/prev-gotestsum.json +server/shard-*.txt diff --git a/server/.go-version b/server/.go-version index 7a429d68a3..e6a6e7cd3e 100644 --- a/server/.go-version +++ b/server/.go-version @@ -1 +1 @@ -1.24.6 +1.25.8 diff --git a/server/Makefile b/server/Makefile index 00688a7fd0..37a9963daa 100644 --- a/server/Makefile +++ b/server/Makefile @@ -299,7 +299,7 @@ golang-versions: ## Install Golang versions used for compatibility testing (e.g. export GO_COMPATIBILITY_TEST_VERSIONS="${GO_COMPATIBILITY_TEST_VERSIONS}" golangci-lint: setup-go-work ## Run golangci-lint on codebase - $(GO) install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 + $(GO) install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.11.4 ifeq ($(BUILD_ENTERPRISE_READY),true) $(GOBIN)/golangci-lint run ./... ./public/... $(BUILD_ENTERPRISE_DIR)/... else @@ -415,7 +415,7 @@ endif check-style: plugin-checker vet golangci-lint ## Runs style/lint checks gotestsum: - $(GO) install gotest.tools/gotestsum@v1.11.0 + $(GO) install gotest.tools/gotestsum@v1.13.0 test-compile: setup-go-work gotestsum ## Compile tests. @echo COMPILE TESTS diff --git a/server/build/docker-compose.common.yml b/server/build/docker-compose.common.yml index b6a05a03a2..87c3f229d0 100644 --- a/server/build/docker-compose.common.yml +++ b/server/build/docker-compose.common.yml @@ -47,7 +47,7 @@ services: - mm-test environment: POSTGRES_USER: mmuser - POSTGRES_PASSWORD: mostest + POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-mostest} POSTGRES_DB: mattermost_test command: postgres -c 'config_file=/etc/postgresql/postgresql.conf' volumes: diff --git a/server/channels/app/app_test.go b/server/channels/app/app_test.go index 80f8284d44..0dc1b2705b 100644 --- a/server/channels/app/app_test.go +++ b/server/channels/app/app_test.go @@ -65,9 +65,9 @@ func TestUnitUpdateConfig(t *testing.T) { require.False(t, th.App.IsConfigReadOnly()) - var called int32 + var called atomic.Int32 th.App.AddConfigListener(func(old, current *model.Config) { - atomic.AddInt32(&called, 1) + called.Add(1) assert.Equal(t, prev, *old.ServiceSettings.SiteURL) assert.Equal(t, "http://foo.com", *current.ServiceSettings.SiteURL) }) @@ -77,7 +77,7 @@ func TestUnitUpdateConfig(t *testing.T) { }) // callback should be called once - assert.Equal(t, int32(1), atomic.LoadInt32(&called)) + assert.Equal(t, int32(1), called.Load()) } func TestDoAdvancedPermissionsMigration(t *testing.T) { diff --git a/server/channels/app/email/email_batching_test.go b/server/channels/app/email/email_batching_test.go index 9ff6d74acc..5dff624fdb 100644 --- a/server/channels/app/email/email_batching_test.go +++ b/server/channels/app/email/email_batching_test.go @@ -129,9 +129,9 @@ func TestCheckPendingNotifications(t *testing.T) { }}) require.NoError(t, nErr) - var wasCalled int32 + var wasCalled atomic.Int32 job.checkPendingNotifications(time.Unix(10050, 0), func(string, []*batchedNotification) { - atomic.StoreInt32(&wasCalled, int32(1)) + wasCalled.Store(int32(1)) }) // A hack to check whether the handler was called. @@ -141,7 +141,7 @@ func TestCheckPendingNotifications(t *testing.T) { // We do a check outside the email handler, because otherwise, failing from // inside the handler doesn't let the .Go() function exit cleanly, and it gets // stuck during server shutdown, trying to wait for the goroutine to exit - require.Equal(t, int32(0), atomic.LoadInt32(&wasCalled), "email handler should not have been called") + require.Equal(t, int32(0), wasCalled.Load(), "email handler should not have been called") require.Nil(t, job.pendingNotifications[th.BasicUser.Id]) require.Empty(t, job.pendingNotifications[th.BasicUser.Id], "should've remove queued post since user acted") diff --git a/server/channels/app/platform/config_test.go b/server/channels/app/platform/config_test.go index 11892b5974..aefd7ced8b 100644 --- a/server/channels/app/platform/config_test.go +++ b/server/channels/app/platform/config_test.go @@ -186,12 +186,9 @@ func TestIsFirstUserAccountThunderingHerd(t *testing.T) { var wg sync.WaitGroup for i := 0; i < te.concurrentRequest; i++ { - wg.Add(1) - - go func() { - defer wg.Done() + wg.Go(func() { require.Equal(t, te.result, th.Service.IsFirstUserAccount()) - }() + }) } wg.Wait() diff --git a/server/channels/app/platform/web_conn.go b/server/channels/app/platform/web_conn.go index 965a5a578e..4fb89e313a 100644 --- a/server/channels/app/platform/web_conn.go +++ b/server/channels/app/platform/web_conn.go @@ -407,11 +407,9 @@ func (wc *WebConn) SetSession(v *model.Session) { // is ready to send/receive messages. func (wc *WebConn) Pump() { var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { wc.writePump() - }() + }) wg.Add(1) go wc.pluginPostedConsumer(&wg) diff --git a/server/channels/app/post_metadata_test.go b/server/channels/app/post_metadata_test.go index 59e50058b5..c5960d762a 100644 --- a/server/channels/app/post_metadata_test.go +++ b/server/channels/app/post_metadata_test.go @@ -520,7 +520,7 @@ func TestPreparePostForClient(t *testing.T) { name: "markdown", link: "[markdown](%s) link", // This is because markdown links are not currently supported in the opengraph fetching code - // if you just implmented this, remove the `notImplmented` field + // if you just implemented this, remove the `notImplmented` field notImplmented: true, }, { diff --git a/server/channels/app/post_test.go b/server/channels/app/post_test.go index 7bb3e59fe5..ed1209b78e 100644 --- a/server/channels/app/post_test.go +++ b/server/channels/app/post_test.go @@ -162,9 +162,7 @@ func TestCreatePostDeduplicate(t *testing.T) { // Launch a goroutine to make the first CreatePost call that will get delayed // by the plugin above. - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { var appErr *model.AppError post, _, appErr = th.App.CreatePostAsUser(th.Context.WithSession(session), &model.Post{ UserId: th.BasicUser.Id, @@ -174,7 +172,7 @@ func TestCreatePostDeduplicate(t *testing.T) { }, session.Id, true) require.Nil(t, appErr) require.Equal(t, post.Message, "plugin delayed") - }() + }) // Give the goroutine above a chance to start and get delayed by the plugin. time.Sleep(2 * time.Second) @@ -3263,12 +3261,10 @@ func TestCollapsedThreadFetch(t *testing.T) { // we introduce a race to trigger an unexpected error from the db side. var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { err := th.Server.Store().Post().PermanentDeleteByUser(th.Context, user1.Id) require.NoError(t, err) - }() + }) require.NotPanics(t, func() { // We're only testing that this doesn't panic, not checking the error diff --git a/server/channels/app/shared_channel_global_user_sync_self_referential_test.go b/server/channels/app/shared_channel_global_user_sync_self_referential_test.go index bcb2fe9773..f8fae2dedb 100644 --- a/server/channels/app/shared_channel_global_user_sync_self_referential_test.go +++ b/server/channels/app/shared_channel_global_user_sync_self_referential_test.go @@ -453,7 +453,7 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { // - Cursor updates only when sync is successful EnsureCleanState(t, th, ss) - var syncAttempts int32 + var syncAttempts atomic.Int32 var failureMode atomic.Bool failureMode.Store(false) var syncHandler *SelfReferentialSyncHandler @@ -462,7 +462,7 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/api/v4/remotecluster/msg": - atomic.AddInt32(&syncAttempts, 1) + syncAttempts.Add(1) if failureMode.Load() { w.WriteHeader(http.StatusInternalServerError) @@ -520,7 +520,7 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { // Wait for first sync require.Eventually(t, func() bool { - return atomic.LoadInt32(&syncAttempts) > 0 + return syncAttempts.Load() > 0 }, 5*time.Second, 100*time.Millisecond, "Should have attempted sync") // Verify cursor was updated @@ -539,13 +539,13 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { require.NoError(t, err) // Second sync - should fail - initialAttempts := atomic.LoadInt32(&syncAttempts) + initialAttempts := syncAttempts.Load() err = service.HandleSyncAllUsersForTesting(selfCluster) require.NoError(t, err) // The method itself shouldn't error, just the remote call // Wait for failed sync attempt require.Eventually(t, func() bool { - return atomic.LoadInt32(&syncAttempts) > initialAttempts + return syncAttempts.Load() > initialAttempts }, 5*time.Second, 100*time.Millisecond, "Should have attempted sync") // Verify cursor was NOT updated on failure @@ -557,13 +557,13 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { failureMode.Store(false) // Third sync - should succeed and update cursor - preSuccessAttempts := atomic.LoadInt32(&syncAttempts) + preSuccessAttempts := syncAttempts.Load() err = service.HandleSyncAllUsersForTesting(selfCluster) require.NoError(t, err) // Wait for successful sync require.Eventually(t, func() bool { - return atomic.LoadInt32(&syncAttempts) > preSuccessAttempts + return syncAttempts.Load() > preSuccessAttempts }, 5*time.Second, 100*time.Millisecond, "Should have attempted sync") // Verify cursor was updated after successful sync @@ -579,12 +579,12 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { // - Ensures cursor is only updated when flag is enabled EnsureCleanState(t, th, ss) - var syncMessageCount int32 + var syncMessageCount atomic.Int32 // Create test HTTP server testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/api/v4/remotecluster/msg" { - atomic.AddInt32(&syncMessageCount, 1) + syncMessageCount.Add(1) } writeOKResponse(w) })) @@ -622,13 +622,13 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { err = th.App.ReloadConfig() require.NoError(t, err) - atomic.StoreInt32(&syncMessageCount, 0) + syncMessageCount.Store(0) err = service.HandleSyncAllUsersForTesting(selfCluster) require.NoError(t, err) // Verify no sync messages were sent require.Never(t, func() bool { - return atomic.LoadInt32(&syncMessageCount) > 0 + return syncMessageCount.Load() > 0 }, 2*time.Second, 100*time.Millisecond, "No sync should occur with feature flag disabled") // Verify cursor was not updated @@ -645,13 +645,13 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { err = th.App.ReloadConfig() require.NoError(t, err) - atomic.StoreInt32(&syncMessageCount, 0) + syncMessageCount.Store(0) err = service.HandleSyncAllUsersForTesting(selfCluster) require.NoError(t, err) // Verify sync messages were sent require.Eventually(t, func() bool { - return atomic.LoadInt32(&syncMessageCount) > 0 + return syncMessageCount.Load() > 0 }, 5*time.Second, 100*time.Millisecond, "Sync should occur with feature flag enabled") // Verify cursor was updated @@ -667,13 +667,13 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { // - Tests cursor updates in both scenarios EnsureCleanState(t, th, ss) - var syncMessageCount int32 + var syncMessageCount atomic.Int32 var connectionOpenSyncOccurred atomic.Bool // Create test HTTP server testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/api/v4/remotecluster/msg" { - atomic.AddInt32(&syncMessageCount, 1) + syncMessageCount.Add(1) // Parse message to check if it's a user sync bodyBytes, _ := io.ReadAll(r.Body) @@ -724,12 +724,12 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { // Verify no automatic sync occurs within a reasonable time require.Never(t, func() bool { - return connectionOpenSyncOccurred.Load() || atomic.LoadInt32(&syncMessageCount) > 0 + return connectionOpenSyncOccurred.Load() || syncMessageCount.Load() > 0 }, 2*time.Second, 100*time.Millisecond, "No automatic sync should occur when config is disabled") // Test 2: Connection open with sync enabled // Reset counters - atomic.StoreInt32(&syncMessageCount, 0) + syncMessageCount.Store(0) connectionOpenSyncOccurred.Store(false) // Enable config option @@ -767,7 +767,7 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { }, 5*time.Second, 100*time.Millisecond, "Automatic sync should occur when config is enabled") // Verify sync occurred - assert.Greater(t, atomic.LoadInt32(&syncMessageCount), int32(0), "Should have sync messages when config enabled") + assert.Greater(t, syncMessageCount.Load(), int32(0), "Should have sync messages when config enabled") // Verify cursor was updated updatedCluster, err2 := ss.RemoteCluster().Get(selfCluster2.RemoteId, true) @@ -849,7 +849,7 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { // - No partial data should be persisted EnsureCleanState(t, th, ss) - var syncAttempts int32 + var syncAttempts atomic.Int32 var serverOnline atomic.Bool serverOnline.Store(true) @@ -862,9 +862,9 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { } if r.URL.Path == "/api/v4/remotecluster/msg" { - atomic.AddInt32(&syncAttempts, 1) + syncAttempts.Add(1) // On second attempt, go offline - if atomic.LoadInt32(&syncAttempts) >= 2 { + if syncAttempts.Load() >= 2 { serverOnline.Store(false) w.WriteHeader(http.StatusServiceUnavailable) return @@ -903,7 +903,7 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { // Wait for first sync require.Eventually(t, func() bool { - return atomic.LoadInt32(&syncAttempts) >= 1 + return syncAttempts.Load() >= 1 }, 5*time.Second, 100*time.Millisecond) // Get cursor after first sync @@ -926,7 +926,7 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) { // Wait for second sync attempt require.Eventually(t, func() bool { - return atomic.LoadInt32(&syncAttempts) >= 2 + return syncAttempts.Load() >= 2 }, 5*time.Second, 100*time.Millisecond) // Verify cursor was not updated after failed sync diff --git a/server/channels/app/shared_channel_membership_sync_self_referential_test.go b/server/channels/app/shared_channel_membership_sync_self_referential_test.go index 1960ba7d8f..656559b8bd 100644 --- a/server/channels/app/shared_channel_membership_sync_self_referential_test.go +++ b/server/channels/app/shared_channel_membership_sync_self_referential_test.go @@ -66,12 +66,12 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // The test ensures that sync messages are sent asynchronously after a minimum delay for both add and remove operations. EnsureCleanState(t, th, ss) // Track sync messages received - var syncMessageCount int32 + var syncMessageCount atomic.Int32 var syncHandler *SelfReferentialSyncHandler // Create a test HTTP server that acts as the "remote" cluster testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - atomic.AddInt32(&syncMessageCount, 1) + syncMessageCount.Add(1) if syncHandler != nil { syncHandler.HandleRequest(w, r) } else { @@ -145,7 +145,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Wait for async sync with more generous timeout (minimum delay is 2 seconds + async task processing) require.Eventually(t, func() bool { - count := atomic.LoadInt32(&syncMessageCount) + count := syncMessageCount.Load() return count > 0 }, 15*time.Second, 200*time.Millisecond, "Should have received at least one sync message via automatic sync") @@ -162,7 +162,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Reset sync counter and wait for background tasks to settle var initialCount int32 require.Eventually(t, func() bool { - initialCount = atomic.LoadInt32(&syncMessageCount) + initialCount = syncMessageCount.Load() return !service.HasPendingTasksForTesting() }, 5*time.Second, 100*time.Millisecond, "Background tasks should settle before removal test") @@ -172,7 +172,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Wait for removal sync with increased timeout require.Eventually(t, func() bool { - count := atomic.LoadInt32(&syncMessageCount) + count := syncMessageCount.Load() return count > initialCount }, 20*time.Second, 200*time.Millisecond, "Should have received sync message for user removal") @@ -534,7 +534,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // 2. No members are synced during failure mode // 3. Once the server recovers, sync completes successfully EnsureCleanState(t, th, ss) - var syncAttempts int32 + var syncAttempts atomic.Int32 var failureMode atomic.Bool failureMode.Store(true) var successfulSyncs []string @@ -543,7 +543,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/api/v4/remotecluster/msg" { - atomic.AddInt32(&syncAttempts, 1) + syncAttempts.Add(1) if failureMode.Load() { w.WriteHeader(http.StatusInternalServerError) @@ -624,11 +624,11 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Wait for first sync attempt with more robust checking require.Eventually(t, func() bool { - attempts := atomic.LoadInt32(&syncAttempts) + attempts := syncAttempts.Load() return attempts > 0 }, 15*time.Second, 100*time.Millisecond, "Should have attempted sync during failure mode") - initialAttempts := atomic.LoadInt32(&syncAttempts) + initialAttempts := syncAttempts.Load() assert.Greater(t, initialAttempts, int32(0), "Should have attempted sync") assert.Empty(t, successfulSyncs, "No successful syncs during failure mode") @@ -650,7 +650,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { }, 15*time.Second, 100*time.Millisecond, "Should have successful sync after recovery") // Verify recovery - finalAttempts := atomic.LoadInt32(&syncAttempts) + finalAttempts := syncAttempts.Load() assert.Greater(t, finalAttempts, initialAttempts, "Should have retried after recovery") }) t.Run("Test 5: Manual sync with cursor management", func(t *testing.T) { @@ -660,9 +660,9 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // 3. Verifies all operations are properly synced and cursor is updated correctly // 4. Validates that the LastMembersSyncAt cursor advances after each sync operation EnsureCleanState(t, th, ss) - var totalSyncMessages int32 - var addOperations int32 - var removeOperations int32 + var totalSyncMessages atomic.Int32 + var addOperations atomic.Int32 + var removeOperations atomic.Int32 var selfCluster *model.RemoteCluster // Create sync handler @@ -686,9 +686,9 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Count membership changes from the unified field for _, change := range syncMsg.MembershipChanges { if change.IsAdd { - atomic.AddInt32(&addOperations, 1) + addOperations.Add(1) } else { - atomic.AddInt32(&removeOperations, 1) + removeOperations.Add(1) } } } @@ -773,10 +773,10 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Wait for initial sync to complete require.Eventually(t, func() bool { - return atomic.LoadInt32(&addOperations) >= 10 + return addOperations.Load() >= 10 }, 10*time.Second, 100*time.Millisecond, "Should sync all initial users") - initialAdds := atomic.LoadInt32(&addOperations) + initialAdds := addOperations.Load() assert.GreaterOrEqual(t, initialAdds, int32(10), "Should sync all initial users") // Verify cursor was updated after initial sync @@ -808,15 +808,15 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { } // Sync mixed changes - previousMessages := atomic.LoadInt32(&totalSyncMessages) + previousMessages := totalSyncMessages.Load() err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) require.NoError(t, err) // Wait for mixed changes sync to complete require.Eventually(t, func() bool { - messages := atomic.LoadInt32(&totalSyncMessages) - removes := atomic.LoadInt32(&removeOperations) + messages := totalSyncMessages.Load() + removes := removeOperations.Load() return messages > previousMessages && removes >= 3 }, 10*time.Second, 100*time.Millisecond, "Should sync mixed changes") @@ -836,9 +836,9 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { expectedMembers := 10 - 3 + 5 + 1 // initial - removed + added + system admin assert.Equal(t, expectedMembers, len(members), "Should have correct final member count") - finalMessages := atomic.LoadInt32(&totalSyncMessages) - finalAdds := atomic.LoadInt32(&addOperations) - finalRemoves := atomic.LoadInt32(&removeOperations) + finalMessages := totalSyncMessages.Load() + finalAdds := addOperations.Load() + finalRemoves := removeOperations.Load() assert.Greater(t, finalMessages, int32(0), "Should have sync messages") assert.Greater(t, finalAdds, int32(0), "Should have add operations") @@ -850,7 +850,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // 2. Changes from one cluster propagate through our server to other clusters // 3. Removals sync to all clusters EnsureCleanState(t, th, ss) - var totalSyncMessages int32 + var totalSyncMessages atomic.Int32 var syncMessagesPerCluster = make(map[string]*int32) // Create multiple test HTTP servers to simulate different remote clusters @@ -997,7 +997,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // This simulates cluster-2 receiving a membership change and propagating it // Reset counters - atomic.StoreInt32(&totalSyncMessages, 0) + totalSyncMessages.Store(0) for _, countPtr := range syncMessagesPerCluster { atomic.StoreInt32(countPtr, 0) } @@ -1058,7 +1058,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Part 3: Test removal syncing to all clusters // Reset counters - atomic.StoreInt32(&totalSyncMessages, 0) + totalSyncMessages.Store(0) for _, countPtr := range syncMessagesPerCluster { atomic.StoreInt32(countPtr, 0) } @@ -1098,7 +1098,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // 2. When the feature flag is enabled, sync messages should be sent as expected // This ensures that the feature can be safely disabled in production without triggering unintended syncs EnsureCleanState(t, th, ss) - var syncMessageCount int32 + var syncMessageCount atomic.Int32 // Disable feature flag from the beginning to prevent any automatic sync os.Setenv("MM_FEATUREFLAGS_ENABLESHAREDCHANNELMEMBERSYNC", "false") @@ -1108,7 +1108,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Create test HTTP server that counts sync messages testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/api/v4/remotecluster/msg" { - atomic.AddInt32(&syncMessageCount, 1) + syncMessageCount.Add(1) } writeOKResponse(w) })) @@ -1168,13 +1168,13 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { require.Nil(t, appErr) } - atomic.StoreInt32(&syncMessageCount, 0) + syncMessageCount.Store(0) err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) require.NoError(t, err) // Verify no sync messages were sent require.Never(t, func() bool { - return atomic.LoadInt32(&syncMessageCount) > 0 + return syncMessageCount.Load() > 0 }, 2*time.Second, 100*time.Millisecond, "No sync should occur with feature flag disabled") // Test 2: Sync with feature flag enabled @@ -1182,13 +1182,13 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { cfg.FeatureFlags.EnableSharedChannelsMemberSync = true }) - atomic.StoreInt32(&syncMessageCount, 0) + syncMessageCount.Store(0) err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) require.NoError(t, err) // Verify sync messages were sent require.Eventually(t, func() bool { - return atomic.LoadInt32(&syncMessageCount) > 0 + return syncMessageCount.Load() > 0 }, 5*time.Second, 100*time.Millisecond, "Sync should occur with feature flag enabled") }) t.Run("Test 8: Sync Task After Connection Becomes Available", func(t *testing.T) { @@ -1307,7 +1307,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // 5. No partial data is persisted from the failed sync EnsureCleanState(t, th, ss) - var syncAttempts int32 + var syncAttempts atomic.Int32 var serverOnline atomic.Bool serverOnline.Store(true) var syncHandler *SelfReferentialSyncHandler @@ -1321,7 +1321,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { } if r.URL.Path == "/api/v4/remotecluster/msg" { - currentAttempt := atomic.AddInt32(&syncAttempts, 1) + currentAttempt := syncAttempts.Add(1) // On second sync cycle, go offline (allow first full sync to complete) if currentAttempt > 2 { serverOnline.Store(false) @@ -1398,7 +1398,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Wait for first sync with more generous timeout require.Eventually(t, func() bool { - return atomic.LoadInt32(&syncAttempts) >= 1 + return syncAttempts.Load() >= 1 }, 15*time.Second, 200*time.Millisecond, "Should complete first sync") // Wait for cursor to be updated after first sync @@ -1427,7 +1427,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Wait for second sync attempt with more generous timeout require.Eventually(t, func() bool { - return atomic.LoadInt32(&syncAttempts) >= 2 + return syncAttempts.Load() >= 2 }, 20*time.Second, 200*time.Millisecond, "Should attempt second sync") // Wait for any cursor updates to complete and verify cursor was not updated @@ -1457,7 +1457,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { var mu sync.Mutex var syncHandler *SelfReferentialSyncHandler var testServer *httptest.Server - var totalSyncMessages int32 + var totalSyncMessages atomic.Int32 // Create users user1 := th.CreateUser() @@ -1528,7 +1528,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { for _, change := range syncMsg.MembershipChanges { if change.IsAdd { syncedChannelUsers[channelId] = append(syncedChannelUsers[channelId], change.UserId) - atomic.AddInt32(&totalSyncMessages, 1) + totalSyncMessages.Add(1) } } } @@ -1583,7 +1583,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Ensure the sync handler is ready by waiting for the first message require.Eventually(t, func() bool { - return atomic.LoadInt32(&totalSyncMessages) > 0 + return totalSyncMessages.Load() > 0 }, 10*time.Second, 50*time.Millisecond, "Expected at least one sync message to be sent") // Calculate expected number of sync messages @@ -1595,7 +1595,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // Wait for all sync messages to be processed with detailed debugging require.Eventually(t, func() bool { - currentMessages := atomic.LoadInt32(&totalSyncMessages) + currentMessages := totalSyncMessages.Load() mu.Lock() channelCount := len(syncedChannelUsers) @@ -1613,7 +1613,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { return currentMessages >= expectedSyncMessages }, 30*time.Second, 200*time.Millisecond, - fmt.Sprintf("Expected %d sync messages, but got %d", expectedSyncMessages, atomic.LoadInt32(&totalSyncMessages))) + fmt.Sprintf("Expected %d sync messages, but got %d", expectedSyncMessages, totalSyncMessages.Load())) // Verify we have complete data for all channels require.Eventually(t, func() bool { @@ -1697,7 +1697,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // EnsureCleanState(t, th, ss) // var syncMessages []model.SyncMsg // var mu sync.Mutex - // var syncMessageCount int32 + // var syncMessageCount atomic.Int32 // var selfCluster *model.RemoteCluster // // Create sync handler @@ -1706,7 +1706,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // // Create test HTTP server that tracks sync messages // testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // if r.URL.Path == "/api/v4/remotecluster/msg" { - // atomic.AddInt32(&syncMessageCount, 1) + // syncMessageCount.Add(1) // // Read body once // bodyBytes, readErr := io.ReadAll(r.Body) @@ -1806,7 +1806,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // // Wait for initial sync to complete // require.Eventually(t, func() bool { - // count := atomic.LoadInt32(&syncMessageCount) + // count := syncMessageCount.Load() // return count > 0 // }, 15*time.Second, 200*time.Millisecond, "Should have initial sync messages") @@ -1853,7 +1853,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // // Phase 5: Conflict resolution sync // // Reset message tracking for conflict resolution phase - // atomic.StoreInt32(&syncMessageCount, 0) + // syncMessageCount.Store(0) // mu.Lock() // syncMessages = []model.SyncMsg{} // mu.Unlock() @@ -1863,7 +1863,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // // Wait for conflict resolution sync to complete // require.Eventually(t, func() bool { - // count := atomic.LoadInt32(&syncMessageCount) + // count := syncMessageCount.Load() // return count > 0 // }, 20*time.Second, 200*time.Millisecond, "Should receive conflict resolution sync messages") @@ -1907,7 +1907,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // require.Nil(t, appErr) // // Reset and sync this new user - // atomic.StoreInt32(&syncMessageCount, 0) + // syncMessageCount.Store(0) // mu.Lock() // syncMessages = []model.SyncMsg{} // mu.Unlock() @@ -1932,7 +1932,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // }, 15*time.Second, 200*time.Millisecond, "New user should be synced correctly after conflict resolution") // // Phase 9: Verify efficiency - no redundant syncs for existing members - // atomic.StoreInt32(&syncMessageCount, 0) + // syncMessageCount.Store(0) // mu.Lock() // syncMessages = []model.SyncMsg{} // mu.Unlock() @@ -1944,7 +1944,7 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { // // Wait for sync completion and verify minimal activity // // Give time for any sync to complete, then check the final count // require.Eventually(t, func() bool { - // finalCount := atomic.LoadInt32(&syncMessageCount) + // finalCount := syncMessageCount.Load() // // Should have minimal activity since all members are already synced // return finalCount <= 1 // }, 10*time.Second, 200*time.Millisecond, "Should have minimal sync activity for already-synced members") diff --git a/server/channels/app/shared_channel_sync_self_referential_utils_test.go b/server/channels/app/shared_channel_sync_self_referential_utils_test.go index d0944f6622..09a7047639 100644 --- a/server/channels/app/shared_channel_sync_self_referential_utils_test.go +++ b/server/channels/app/shared_channel_sync_self_referential_utils_test.go @@ -41,7 +41,7 @@ type SelfReferentialSyncHandler struct { t *testing.T service *sharedchannel.Service selfCluster *model.RemoteCluster - syncMessageCount *int32 + syncMessageCount *atomic.Int32 SimulateUnshared bool // When true, always return ErrChannelIsNotShared for sync messages // Callbacks for capturing sync data @@ -52,12 +52,11 @@ type SelfReferentialSyncHandler struct { // NewSelfReferentialSyncHandler creates a new handler for processing sync messages in tests func NewSelfReferentialSyncHandler(t *testing.T, service *sharedchannel.Service, selfCluster *model.RemoteCluster) *SelfReferentialSyncHandler { - count := int32(0) return &SelfReferentialSyncHandler{ t: t, service: service, selfCluster: selfCluster, - syncMessageCount: &count, + syncMessageCount: &atomic.Int32{}, } } @@ -69,7 +68,7 @@ func NewSelfReferentialSyncHandler(t *testing.T, service *sharedchannel.Service, func (h *SelfReferentialSyncHandler) HandleRequest(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/api/v4/remotecluster/msg": - currentCall := atomic.AddInt32(h.syncMessageCount, 1) + currentCall := h.syncMessageCount.Add(1) // Read and process the sync message body, _ := io.ReadAll(r.Body) @@ -162,7 +161,7 @@ func (h *SelfReferentialSyncHandler) HandleRequest(w http.ResponseWriter, r *htt // GetSyncMessageCount returns the current count of sync messages received func (h *SelfReferentialSyncHandler) GetSyncMessageCount() int32 { - return atomic.LoadInt32(h.syncMessageCount) + return h.syncMessageCount.Load() } // EnsureCleanState ensures a clean test state by removing all shared channels, remote clusters, diff --git a/server/channels/app/support_packet.go b/server/channels/app/support_packet.go index af0a30b4e8..38a1500914 100644 --- a/server/channels/app/support_packet.go +++ b/server/channels/app/support_packet.go @@ -38,10 +38,7 @@ func (a *App) GenerateSupportPacket(rctx request.CTX, options *model.SupportPack mut sync.Mutex // Protects warnings and fileDatas ) - wg.Add(1) - go func() { - defer wg.Done() - + wg.Go(func() { for name, fn := range functions { fileData, err := fn(rctx) mut.Lock() @@ -82,14 +79,11 @@ func (a *App) GenerateSupportPacket(rctx request.CTX, options *model.SupportPack } } mut.Unlock() - }() + }) // Run the cluster generation in a separate goroutine as CPU profile generation and file upload can take a long time if cluster := a.Cluster(); cluster != nil && *a.Config().ClusterSettings.Enable { - wg.Add(1) - go func() { - defer wg.Done() - + wg.Go(func() { files, err := cluster.GenerateSupportPacket(rctx, options) mut.Lock() if err != nil { @@ -101,7 +95,7 @@ func (a *App) GenerateSupportPacket(rctx request.CTX, options *model.SupportPack fileDatas = append(fileDatas, node...) } mut.Unlock() - }() + }) } wg.Wait() diff --git a/server/channels/app/user_agent.go b/server/channels/app/user_agent.go index 9e9a3143b1..c3aeff9fbc 100644 --- a/server/channels/app/user_agent.go +++ b/server/channels/app/user_agent.go @@ -116,9 +116,8 @@ var versionPrefixes = []string{ func getBrowserVersion(ua *uasurfer.UserAgent, userAgentString string) string { for _, prefix := range versionPrefixes { - if index := strings.Index(userAgentString, prefix); index != -1 { - afterPrefix := userAgentString[index+len(prefix):] - if fields := strings.Fields(afterPrefix); len(fields) > 0 { + if _, after, ok := strings.Cut(userAgentString, prefix); ok { + if fields := strings.Fields(after); len(fields) > 0 { // MM-55320: limitStringLength prevents potential DOS caused by filling an unbounded string with junk data return limitStringLength(fields[0], maxUserAgentVersionLength) } diff --git a/server/channels/jobs/migrations/worker.go b/server/channels/jobs/migrations/worker.go index 5d8eb8681e..5d8c63f1aa 100644 --- a/server/channels/jobs/migrations/worker.go +++ b/server/channels/jobs/migrations/worker.go @@ -28,7 +28,7 @@ type Worker struct { jobServer *jobs.JobServer logger mlog.LoggerIFace store store.Store - closed int32 + closed atomic.Int32 } func MakeWorker(jobServer *jobs.JobServer, store store.Store) *Worker { @@ -48,7 +48,7 @@ func MakeWorker(jobServer *jobs.JobServer, store store.Store) *Worker { func (worker *Worker) Run() { // Set to open if closed before. We are not bothered about multiple opens. - if atomic.CompareAndSwapInt32(&worker.closed, 1, 0) { + if worker.closed.CompareAndSwap(1, 0) { worker.stop = make(chan struct{}) } worker.logger.Debug("Worker started") @@ -71,7 +71,7 @@ func (worker *Worker) Run() { func (worker *Worker) Stop() { // Set to close, and if already closed before, then return. - if !atomic.CompareAndSwapInt32(&worker.closed, 0, 1) { + if !worker.closed.CompareAndSwap(0, 1) { return } worker.logger.Debug("Worker stopping") diff --git a/server/channels/store/storetest/channel_store.go b/server/channels/store/storetest/channel_store.go index d55acfb058..214320cec3 100644 --- a/server/channels/store/storetest/channel_store.go +++ b/server/channels/store/storetest/channel_store.go @@ -44,6 +44,20 @@ type SqlXExecutor interface { Select(dest any, query string, args ...any) error } +// cleanupChannelStoreData purges all channel-related data written by TestChannelStore +// sub-tests. The integrity tests (TestCheck*) do full-table scans and fail if any +// orphaned rows remain. A blanket purge is safe: no FK constraints are enforced in the +// schema, and every test suite creates its own data independently. +func cleanupChannelStoreData(t *testing.T, s SqlStore) { + t.Helper() + db := s.GetMaster() + db.Exec(`DELETE FROM Threads`) + db.Exec(`DELETE FROM ChannelMemberHistory`) + db.Exec(`DELETE FROM ChannelMembers`) + db.Exec(`DELETE FROM Channels`) + db.Exec(`DELETE FROM TeamMembers`) +} + func cleanupChannels(t *testing.T, rctx request.CTX, ss store.Store) { list, err := ss.Channel().GetAllChannels(0, 100000, store.ChannelSearchOpts{IncludeDeleted: true}) require.NoError(t, err, "error cleaning all channels", err) @@ -69,6 +83,7 @@ func channelMemberToJSON(t *testing.T, cm *model.ChannelMember) string { func TestChannelStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { createDefaultRoles(ss) + t.Cleanup(func() { cleanupChannelStoreData(t, s) }) t.Run("Save", func(t *testing.T) { testChannelStoreSave(t, rctx, ss) }) t.Run("SaveDirectChannel", func(t *testing.T) { testChannelStoreSaveDirectChannel(t, rctx, ss, s) }) diff --git a/server/channels/store/storetest/channel_store_categories.go b/server/channels/store/storetest/channel_store_categories.go index 6bf0f41a18..85eab3c1dd 100644 --- a/server/channels/store/storetest/channel_store_categories.go +++ b/server/channels/store/storetest/channel_store_categories.go @@ -149,14 +149,10 @@ func testCreateInitialSidebarCategories(t *testing.T, rctx request.CTX, ss store var wg sync.WaitGroup - for i := 0; i < 10; i++ { - wg.Add(1) - - go func() { - defer wg.Done() - + for range 10 { + wg.Go(func() { _, _ = ss.Channel().CreateInitialSidebarCategories(rctx, userID, team.Id) - }() + }) } wg.Wait() @@ -2298,15 +2294,13 @@ func doTestSidebarCategoryConcurrentAccess(t *testing.T, rctx request.CTX, ss st // Run concurrent operations var wg sync.WaitGroup - for i := 0; i < numGoroutines; i++ { - wg.Add(1) + for i := range numGoroutines { // Run GetSidebarCategoriesForTeamForUser - go func() { - defer wg.Done() + wg.Go(func() { categories, getErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userID, team.Id) require.NoError(t, getErr) require.NotEmpty(t, categories.Categories) - }() + }) // Run UpdateSidebarCategories with different update patterns wg.Add(1) diff --git a/server/channels/web/webhook_test.go b/server/channels/web/webhook_test.go index dafdb43ee0..afbbac0c65 100644 --- a/server/channels/web/webhook_test.go +++ b/server/channels/web/webhook_test.go @@ -30,10 +30,11 @@ func TestIncomingWebhook(t *testing.T) { url := apiClient.URL + "/hooks/" + hook.Id - tooLongText := "" - for i := 0; i < 8200; i++ { - tooLongText += "a" + var tooLongTextBuilder strings.Builder + for range 8200 { + tooLongTextBuilder.WriteString("a") } + tooLongText := tooLongTextBuilder.String() t.Run("WebhookBasics", func(t *testing.T) { payload := "payload={\"text\": \"test text\"}" diff --git a/server/enterprise/elasticsearch/elasticsearch/elasticsearch.go b/server/enterprise/elasticsearch/elasticsearch/elasticsearch.go index 6370ad7c92..4871760b5d 100644 --- a/server/enterprise/elasticsearch/elasticsearch/elasticsearch.go +++ b/server/enterprise/elasticsearch/elasticsearch/elasticsearch.go @@ -37,7 +37,7 @@ var ( type ElasticsearchInterfaceImpl struct { client *elastic.TypedClient mutex sync.RWMutex - ready int32 + ready atomic.Int32 version int fullVersion string plugins []string @@ -67,7 +67,7 @@ func (es *ElasticsearchInterfaceImpl) IsEnabled() bool { } func (es *ElasticsearchInterfaceImpl) IsActive() bool { - return *es.Platform.Config().ElasticsearchSettings.EnableIndexing && atomic.LoadInt32(&es.ready) == 1 + return *es.Platform.Config().ElasticsearchSettings.EnableIndexing && es.ready.Load() == 1 } func (es *ElasticsearchInterfaceImpl) IsIndexingEnabled() bool { @@ -94,7 +94,7 @@ func (es *ElasticsearchInterfaceImpl) Start() *model.AppError { es.mutex.Lock() defer es.mutex.Unlock() - if atomic.LoadInt32(&es.ready) != 0 { + if es.ready.Load() != 0 { // Elasticsearch is already started. We don't return an error // because "Test Connection" already re-initializes the client. So this // can be a valid scenario. @@ -165,7 +165,7 @@ func (es *ElasticsearchInterfaceImpl) Start() *model.AppError { return model.NewAppError("Elasticsearch.start", "ent.elasticsearch.create_template_file_info_if_not_exists.template_create_failed", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError).Wrap(err) } - atomic.StoreInt32(&es.ready, 1) + es.ready.Store(1) return nil } @@ -174,7 +174,7 @@ func (es *ElasticsearchInterfaceImpl) Stop() *model.AppError { es.mutex.Lock() defer es.mutex.Unlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.start", "ent.elasticsearch.stop.already_stopped.app_error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -187,7 +187,7 @@ func (es *ElasticsearchInterfaceImpl) Stop() *model.AppError { es.bulkProcessor = nil } - atomic.StoreInt32(&es.ready, 0) + es.ready.Store(0) return nil } @@ -208,7 +208,7 @@ func (es *ElasticsearchInterfaceImpl) IndexPost(post *model.Post, teamId string) es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.IndexPost", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -270,7 +270,7 @@ func (es *ElasticsearchInterfaceImpl) SearchPosts(channels model.ChannelList, se es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return []string{}, nil, model.NewAppError("Elasticsearch.SearchPosts", "ent.elasticsearch.search_posts.disabled", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -594,7 +594,7 @@ func (es *ElasticsearchInterfaceImpl) DeletePost(post *model.Post) *model.AppErr es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.DeletePost", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -616,7 +616,7 @@ func (es *ElasticsearchInterfaceImpl) DeleteChannelPosts(rctx request.CTX, chann es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.DeleteChannelPosts", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -652,7 +652,7 @@ func (es *ElasticsearchInterfaceImpl) DeleteUserPosts(rctx request.CTX, userID s es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.DeleteUserPosts", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -711,7 +711,7 @@ func (es *ElasticsearchInterfaceImpl) IndexChannel(rctx request.CTX, channel *mo es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.IndexChannel", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -752,7 +752,7 @@ func (es *ElasticsearchInterfaceImpl) SearchChannels(teamId, userID string, term es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return []string{}, model.NewAppError("Elasticsearch.SearchChannels", "ent.elasticsearch.search_channels.disabled", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -855,7 +855,7 @@ func (es *ElasticsearchInterfaceImpl) DeleteChannel(channel *model.Channel) *mod es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.DeleteChannel", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -886,7 +886,7 @@ func (es *ElasticsearchInterfaceImpl) IndexUser(rctx request.CTX, user *model.Us es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.IndexUser", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -928,7 +928,7 @@ func (es *ElasticsearchInterfaceImpl) autocompleteUsers(contextCategory string, es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return nil, model.NewAppError("Elasticsearch.autocompleteUsers", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -1041,7 +1041,7 @@ func (es *ElasticsearchInterfaceImpl) autocompleteUsersNotInChannel(teamId, chan es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return nil, model.NewAppError("Elasticsearch.autocompleteUsersNotInChannel", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -1204,7 +1204,7 @@ func (es *ElasticsearchInterfaceImpl) DeleteUser(user *model.User) *model.AppErr es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.DeleteUser", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -1251,7 +1251,7 @@ func (es *ElasticsearchInterfaceImpl) TestConfig(rctx request.CTX, cfg *model.Co } // Resetting the state. - if atomic.CompareAndSwapInt32(&es.ready, 0, 1) { + if es.ready.CompareAndSwap(0, 1) { // Re-assign the client. // This is necessary in case elasticsearch was started // after server start. @@ -1271,7 +1271,7 @@ func (es *ElasticsearchInterfaceImpl) PurgeIndexes(rctx request.CTX) *model.AppE return model.NewAppError("Elasticsearch.PurgeIndexes", "ent.elasticsearch.test_config.license.error", nil, "", http.StatusNotImplemented) } - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.PurgeIndexes", "ent.elasticsearch.generic.disabled", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -1320,7 +1320,7 @@ func (es *ElasticsearchInterfaceImpl) PurgeIndexList(rctx request.CTX, indexes [ return model.NewAppError("Elasticsearch.PurgeIndexList", "ent.elasticsearch.test_config.license.error", nil, "", http.StatusNotImplemented) } - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.PurgeIndexList", "ent.elasticsearch.generic.disabled", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -1389,7 +1389,7 @@ func (es *ElasticsearchInterfaceImpl) DataRetentionDeleteIndexes(rctx request.CT return model.NewAppError("Elasticsearch.DataRetentionDeleteIndexes", "ent.elasticsearch.test_config.license.error", nil, "", http.StatusNotImplemented) } - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.DataRetentionDeleteIndexes", "ent.elasticsearch.generic.disabled", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -1418,7 +1418,7 @@ func (es *ElasticsearchInterfaceImpl) IndexFile(file *model.FileInfo, channelId es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.IndexFile", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -1459,7 +1459,7 @@ func (es *ElasticsearchInterfaceImpl) SearchFiles(channels model.ChannelList, se es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return []string{}, model.NewAppError("Elasticsearch.SearchPosts", "ent.elasticsearch.search_files.disabled", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -1700,7 +1700,7 @@ func (es *ElasticsearchInterfaceImpl) DeleteFile(fileID string) *model.AppError es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.DeleteFile", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -1731,7 +1731,7 @@ func (es *ElasticsearchInterfaceImpl) DeleteUserFiles(rctx request.CTX, userID s es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.DeleteFilesBatch", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -1763,7 +1763,7 @@ func (es *ElasticsearchInterfaceImpl) DeletePostFiles(rctx request.CTX, postID s es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.DeleteFilesBatch", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } @@ -1794,7 +1794,7 @@ func (es *ElasticsearchInterfaceImpl) DeleteFilesBatch(rctx request.CTX, endTime es.mutex.RLock() defer es.mutex.RUnlock() - if atomic.LoadInt32(&es.ready) == 0 { + if es.ready.Load() == 0 { return model.NewAppError("Elasticsearch.DeleteFilesBatch", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsESBackend}, "", http.StatusInternalServerError) } diff --git a/server/enterprise/elasticsearch/opensearch/opensearch.go b/server/enterprise/elasticsearch/opensearch/opensearch.go index 99c1b56d66..6e5ae4b8e9 100644 --- a/server/enterprise/elasticsearch/opensearch/opensearch.go +++ b/server/enterprise/elasticsearch/opensearch/opensearch.go @@ -39,7 +39,7 @@ var ( type OpensearchInterfaceImpl struct { client *opensearchapi.Client mutex sync.RWMutex - ready int32 + ready atomic.Int32 version int fullVersion string plugins []string @@ -69,7 +69,7 @@ func (os *OpensearchInterfaceImpl) IsEnabled() bool { } func (os *OpensearchInterfaceImpl) IsActive() bool { - return *os.Platform.Config().ElasticsearchSettings.EnableIndexing && atomic.LoadInt32(&os.ready) == 1 + return *os.Platform.Config().ElasticsearchSettings.EnableIndexing && os.ready.Load() == 1 } func (os *OpensearchInterfaceImpl) IsIndexingEnabled() bool { @@ -96,7 +96,7 @@ func (os *OpensearchInterfaceImpl) Start() *model.AppError { os.mutex.Lock() defer os.mutex.Unlock() - if atomic.LoadInt32(&os.ready) != 0 { + if os.ready.Load() != 0 { // Elasticsearch is already started. We don't return an error // because "Test Connection" already re-initializes the client. So this // can be a valid scenario. @@ -187,7 +187,7 @@ func (os *OpensearchInterfaceImpl) Start() *model.AppError { return model.NewAppError("Opensearch.start", "ent.elasticsearch.create_template_file_info_if_not_exists.template_create_failed", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError).Wrap(err) } - atomic.StoreInt32(&os.ready, 1) + os.ready.Store(1) return nil } @@ -196,7 +196,7 @@ func (os *OpensearchInterfaceImpl) Stop() *model.AppError { os.mutex.Lock() defer os.mutex.Unlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.start", "ent.elasticsearch.stop.already_stopped.app_error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -209,7 +209,7 @@ func (os *OpensearchInterfaceImpl) Stop() *model.AppError { } os.client = nil - atomic.StoreInt32(&os.ready, 0) + os.ready.Store(0) return nil } @@ -230,7 +230,7 @@ func (os *OpensearchInterfaceImpl) IndexPost(post *model.Post, teamId string) *m os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.IndexPost", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -300,7 +300,7 @@ func (os *OpensearchInterfaceImpl) SearchPosts(channels model.ChannelList, searc os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return []string{}, nil, model.NewAppError("Opensearch.SearchPosts", "ent.elasticsearch.search_posts.disabled", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -658,7 +658,7 @@ func (os *OpensearchInterfaceImpl) DeletePost(post *model.Post) *model.AppError os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.DeletePost", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -680,7 +680,7 @@ func (os *OpensearchInterfaceImpl) DeleteChannelPosts(rctx request.CTX, channelI os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.DeleteChannelPosts", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -721,7 +721,7 @@ func (os *OpensearchInterfaceImpl) DeleteUserPosts(rctx request.CTX, userID stri os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.DeleteUserPosts", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -787,7 +787,7 @@ func (os *OpensearchInterfaceImpl) IndexChannel(rctx request.CTX, channel *model os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.IndexChannel", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -834,7 +834,7 @@ func (os *OpensearchInterfaceImpl) SearchChannels(teamId, userID string, term st os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return []string{}, model.NewAppError("Opensearch.SearchChannels", "ent.elasticsearch.search_channels.disabled", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -941,7 +941,7 @@ func (os *OpensearchInterfaceImpl) DeleteChannel(channel *model.Channel) *model. os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.DeleteChannel", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -974,7 +974,7 @@ func (os *OpensearchInterfaceImpl) IndexUser(rctx request.CTX, user *model.User, os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.IndexUser", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1022,7 +1022,7 @@ func (os *OpensearchInterfaceImpl) autocompleteUsers(contextCategory string, cat os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return nil, model.NewAppError("Opensearch.autocompleteUsers", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1141,7 +1141,7 @@ func (os *OpensearchInterfaceImpl) autocompleteUsersNotInChannel(teamId, channel os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return nil, model.NewAppError("Opensearch.autocompleteUsersNotInChannel", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1310,7 +1310,7 @@ func (os *OpensearchInterfaceImpl) DeleteUser(user *model.User) *model.AppError os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.DeleteUser", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1359,7 +1359,7 @@ func (os *OpensearchInterfaceImpl) TestConfig(rctx request.CTX, cfg *model.Confi } // Resetting the state. - if atomic.CompareAndSwapInt32(&os.ready, 0, 1) { + if os.ready.CompareAndSwap(0, 1) { // Re-assign the client. // This is necessary in case opensearch was started // after server start. @@ -1379,7 +1379,7 @@ func (os *OpensearchInterfaceImpl) PurgeIndexes(rctx request.CTX) *model.AppErro return model.NewAppError("Opensearch.PurgeIndexes", "ent.elasticsearch.test_config.license.error", nil, "", http.StatusNotImplemented) } - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.PurgeIndexes", "ent.elasticsearch.generic.disabled", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1432,7 +1432,7 @@ func (os *OpensearchInterfaceImpl) PurgeIndexList(rctx request.CTX, indexes []st return model.NewAppError("Opensearch.PurgeIndexList", "ent.elasticsearch.test_config.license.error", nil, "", http.StatusNotImplemented) } - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.PurgeIndexList", "ent.elasticsearch.generic.disabled", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1503,7 +1503,7 @@ func (os *OpensearchInterfaceImpl) DataRetentionDeleteIndexes(rctx request.CTX, return model.NewAppError("Opensearch.DataRetentionDeleteIndexes", "ent.elasticsearch.test_config.license.error", nil, "", http.StatusNotImplemented) } - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.DataRetentionDeleteIndexes", "ent.elasticsearch.generic.disabled", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1536,7 +1536,7 @@ func (os *OpensearchInterfaceImpl) IndexFile(file *model.FileInfo, channelId str os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.IndexFile", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1583,7 +1583,7 @@ func (os *OpensearchInterfaceImpl) SearchFiles(channels model.ChannelList, searc os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return []string{}, model.NewAppError("Opensearch.SearchPosts", "ent.elasticsearch.search_files.disabled", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1830,7 +1830,7 @@ func (os *OpensearchInterfaceImpl) DeleteFile(fileID string) *model.AppError { os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.DeleteFile", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1863,7 +1863,7 @@ func (os *OpensearchInterfaceImpl) DeleteUserFiles(rctx request.CTX, userID stri os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.DeleteFilesBatch", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1900,7 +1900,7 @@ func (os *OpensearchInterfaceImpl) DeletePostFiles(rctx request.CTX, postID stri os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.DeleteFilesBatch", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } @@ -1936,7 +1936,7 @@ func (os *OpensearchInterfaceImpl) DeleteFilesBatch(rctx request.CTX, endTime, l os.mutex.RLock() defer os.mutex.RUnlock() - if atomic.LoadInt32(&os.ready) == 0 { + if os.ready.Load() == 0 { return model.NewAppError("Opensearch.DeleteFilesBatch", "ent.elasticsearch.not_started.error", map[string]any{"Backend": model.ElasticsearchSettingsOSBackend}, "", http.StatusInternalServerError) } diff --git a/server/enterprise/metrics/metrics.go b/server/enterprise/metrics/metrics.go index 8e97ba0421..68cdbf7fbb 100644 --- a/server/enterprise/metrics/metrics.go +++ b/server/enterprise/metrics/metrics.go @@ -2206,12 +2206,12 @@ func extractDBCluster(driver, connectionString string) (string, error) { return "", err } - clusterEnd := strings.Index(host, ".") - if clusterEnd == -1 { + cluster, _, found := strings.Cut(host, ".") + if !found { return host, nil } - return host[:clusterEnd], nil + return cluster, nil } func extractHost(driver, connectionString string) (string, error) { diff --git a/server/go.mod b/server/go.mod index 212c1af912..71f53e11c0 100644 --- a/server/go.mod +++ b/server/go.mod @@ -1,6 +1,6 @@ module github.com/mattermost/mattermost/server/v8 -go 1.24.6 +go 1.25.8 require ( code.sajari.com/docconv/v2 v2.0.0-pre.4 diff --git a/server/platform/services/remotecluster/send_test.go b/server/platform/services/remotecluster/send_test.go index f4407a0da8..e8ef67576a 100644 --- a/server/platform/services/remotecluster/send_test.go +++ b/server/platform/services/remotecluster/send_test.go @@ -37,8 +37,8 @@ func TestBroadcastMsg(t *testing.T) { disablePing = true t.Run("No error", func(t *testing.T) { - var countCallbacks int32 - var countWebReq int32 + var countCallbacks atomic.Int32 + var countWebReq atomic.Int32 merr := merror.New() ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -53,7 +53,7 @@ func TestBroadcastMsg(t *testing.T) { w.Write(b) }() - atomic.AddInt32(&countWebReq, 1) + countWebReq.Add(1) var frame model.RemoteClusterFrame jsonErr := json.NewDecoder(r.Body).Decode(&frame) @@ -102,7 +102,7 @@ func TestBroadcastMsg(t *testing.T) { err = service.BroadcastMsg(ctx, msg, func(msg model.RemoteClusterMsg, remote *model.RemoteCluster, resp *Response, err error) { defer wg.Done() - atomic.AddInt32(&countCallbacks, 1) + countCallbacks.Add(1) if err != nil { merr.Append(err) @@ -127,10 +127,10 @@ func TestBroadcastMsg(t *testing.T) { assert.NoError(t, merr.ErrorOrNil()) - assert.Equal(t, int32(NumRemotes), atomic.LoadInt32(&countCallbacks)) - assert.Equal(t, int32(NumRemotes), atomic.LoadInt32(&countWebReq)) + assert.Equal(t, int32(NumRemotes), countCallbacks.Load()) + assert.Equal(t, int32(NumRemotes), countWebReq.Load()) t.Logf("%d callbacks counted; %d web requests counted; %d expected", - atomic.LoadInt32(&countCallbacks), atomic.LoadInt32(&countWebReq), NumRemotes) + countCallbacks.Load(), countWebReq.Load(), NumRemotes) }) t.Run("HTTP error", func(t *testing.T) { @@ -150,24 +150,24 @@ func TestBroadcastMsg(t *testing.T) { defer service.Shutdown() msg := makeRemoteClusterMsg(msgId, NoteContent) - var countCallbacks int32 - var countErrors int32 + var countCallbacks atomic.Int32 + var countErrors atomic.Int32 wg := &sync.WaitGroup{} wg.Add(NumRemotes) err = service.BroadcastMsg(context.Background(), msg, func(msg model.RemoteClusterMsg, remote *model.RemoteCluster, resp *Response, err error) { defer wg.Done() - atomic.AddInt32(&countCallbacks, 1) + countCallbacks.Add(1) if err != nil { - atomic.AddInt32(&countErrors, 1) + countErrors.Add(1) } }) assert.NoError(t, err) wg.Wait() - assert.Equal(t, int32(NumRemotes), atomic.LoadInt32(&countCallbacks)) - assert.Equal(t, int32(NumRemotes), atomic.LoadInt32(&countErrors)) + assert.Equal(t, int32(NumRemotes), countCallbacks.Load()) + assert.Equal(t, int32(NumRemotes), countErrors.Load()) }) } diff --git a/server/platform/services/remotecluster/service_test.go b/server/platform/services/remotecluster/service_test.go index 1f0bff10c2..4491c26b02 100644 --- a/server/platform/services/remotecluster/service_test.go +++ b/server/platform/services/remotecluster/service_test.go @@ -14,18 +14,18 @@ import ( ) func TestService_AddTopicListener(t *testing.T) { - var count int32 + var count atomic.Int32 l1 := func(msg model.RemoteClusterMsg, rc *model.RemoteCluster, resp *Response) error { - atomic.AddInt32(&count, 1) + count.Add(1) return nil } l2 := func(msg model.RemoteClusterMsg, rc *model.RemoteCluster, resp *Response) error { - atomic.AddInt32(&count, 1) + count.Add(1) return nil } l3 := func(msg model.RemoteClusterMsg, rc *model.RemoteCluster, resp *Response) error { - atomic.AddInt32(&count, 1) + count.Add(1) return nil } @@ -47,26 +47,26 @@ func TestService_AddTopicListener(t *testing.T) { msg2 := model.RemoteClusterMsg{Topic: "different"} service.ReceiveIncomingMsg(rc, msg1) - assert.Equal(t, int32(2), atomic.LoadInt32(&count)) + assert.Equal(t, int32(2), count.Load()) service.ReceiveIncomingMsg(rc, msg2) - assert.Equal(t, int32(3), atomic.LoadInt32(&count)) + assert.Equal(t, int32(3), count.Load()) service.RemoveTopicListener(l1id) service.ReceiveIncomingMsg(rc, msg1) - assert.Equal(t, int32(4), atomic.LoadInt32(&count)) + assert.Equal(t, int32(4), count.Load()) service.RemoveTopicListener(l2id) service.ReceiveIncomingMsg(rc, msg1) - assert.Equal(t, int32(4), atomic.LoadInt32(&count)) + assert.Equal(t, int32(4), count.Load()) service.ReceiveIncomingMsg(rc, msg2) - assert.Equal(t, int32(5), atomic.LoadInt32(&count)) + assert.Equal(t, int32(5), count.Load()) service.RemoveTopicListener(l3id) service.ReceiveIncomingMsg(rc, msg1) service.ReceiveIncomingMsg(rc, msg2) - assert.Equal(t, int32(5), atomic.LoadInt32(&count)) + assert.Equal(t, int32(5), count.Load()) listeners = service.getTopicListeners("test") assert.Empty(t, listeners) diff --git a/server/public/go.mod b/server/public/go.mod index 25239bb2c4..907031379b 100644 --- a/server/public/go.mod +++ b/server/public/go.mod @@ -1,6 +1,6 @@ module github.com/mattermost/mattermost/server/public -go 1.24.6 +go 1.25.8 require ( github.com/blang/semver/v4 v4.0.0 diff --git a/server/public/model/content_flagging_settings.go b/server/public/model/content_flagging_settings.go index c2b7e60ff1..a1e67dfe22 100644 --- a/server/public/model/content_flagging_settings.go +++ b/server/public/model/content_flagging_settings.go @@ -65,7 +65,7 @@ func (cfs *ContentFlaggingNotificationSettings) IsValid() *AppError { } } - if cfs.EventTargetMapping[EventFlagged] == nil || len(cfs.EventTargetMapping[EventFlagged]) == 0 { + if len(cfs.EventTargetMapping[EventFlagged]) == 0 { return NewAppError("Config.IsValid", "model.config.is_valid.notification_settings.reviewer_flagged_notification_disabled", nil, "", http.StatusBadRequest) } diff --git a/server/public/model/push_notification.go b/server/public/model/push_notification.go index 31823a0f46..4b5d883d14 100644 --- a/server/public/model/push_notification.go +++ b/server/public/model/push_notification.go @@ -86,10 +86,8 @@ func (pn *PushNotification) DeepCopy() *PushNotification { } func (pn *PushNotification) SetDeviceIdAndPlatform(deviceId string) { - index := strings.Index(deviceId, ":") - - if index > -1 { - pn.Platform = deviceId[:index] - pn.DeviceId = deviceId[index+1:] + if platform, id, ok := strings.Cut(deviceId, ":"); ok { + pn.Platform = platform + pn.DeviceId = id } } diff --git a/server/public/plugin/client_rpc.go b/server/public/plugin/client_rpc.go index bba1e3265a..89f93200d6 100644 --- a/server/public/plugin/client_rpc.go +++ b/server/public/plugin/client_rpc.go @@ -245,23 +245,19 @@ type Z_OnActivateReturns struct { func (g *hooksRPCClient) OnActivate() error { muxId := g.muxBroker.NextId() - g.doneWg.Add(1) - go func() { - defer g.doneWg.Done() + g.doneWg.Go(func() { g.muxBroker.AcceptAndServe(muxId, &apiRPCServer{ impl: g.apiImpl, muxBroker: g.muxBroker, }) - }() + }) nextID := g.muxBroker.NextId() - g.doneWg.Add(1) - go func() { - defer g.doneWg.Done() + g.doneWg.Go(func() { g.muxBroker.AcceptAndServe(nextID, &dbRPCServer{ dbImpl: g.driver, }) - }() + }) _args := &Z_OnActivateArgs{ APIMuxId: muxId, diff --git a/server/public/pluginapi/cluster/job_test.go b/server/public/pluginapi/cluster/job_test.go index 49019ae800..05607264ba 100644 --- a/server/public/pluginapi/cluster/job_test.go +++ b/server/public/pluginapi/cluster/job_test.go @@ -326,12 +326,10 @@ func TestSchedule(t *testing.T) { var wg sync.WaitGroup for i := 0; i < 3; i++ { job := jobs[i] - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { err := job.Close() require.NoError(t, err) - }() + }) } wg.Wait() @@ -386,12 +384,10 @@ func TestSchedule(t *testing.T) { var wg sync.WaitGroup for i := 0; i < 3; i++ { job := jobs[i] - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { err := job.Close() require.NoError(t, err) - }() + }) } wg.Wait() diff --git a/server/public/pluginapi/kv_memory_test.go b/server/public/pluginapi/kv_memory_test.go index 12b131e060..032edd54b5 100644 --- a/server/public/pluginapi/kv_memory_test.go +++ b/server/public/pluginapi/kv_memory_test.go @@ -135,15 +135,12 @@ func TestMemoryStoreSet(t *testing.T) { store := pluginapi.MemoryStore{} var wg sync.WaitGroup const n = 100 - for i := 0; i < n; i++ { - i := i - wg.Add(1) - go func() { - defer wg.Done() + for i := range n { + wg.Go(func() { ok, err := store.Set(fmt.Sprintf("k_%d", i), []byte("value")) require.NoError(t, err) require.True(t, ok) - }() + }) } wg.Wait() @@ -189,14 +186,11 @@ func TestMemoryStoreSetAtomicWithRetries(t *testing.T) { store := pluginapi.MemoryStore{} var wg sync.WaitGroup const n = 10 - for i := 0; i < n; i++ { - i := i - wg.Add(1) - go func() { - defer wg.Done() + for i := range n { + wg.Go(func() { err := store.SetAtomicWithRetries("key", func(oldValue []byte) (any, error) { return fmt.Sprintf("k_%d", i), nil }) require.NoError(t, err) - }() + }) } wg.Wait() diff --git a/server/scripts/run-shard-tests.sh b/server/scripts/run-shard-tests.sh new file mode 100755 index 0000000000..f687d97f29 --- /dev/null +++ b/server/scripts/run-shard-tests.sh @@ -0,0 +1,116 @@ +#!/bin/bash +set -uo pipefail + +# run-shard-tests.sh — Multi-run test wrapper for sharded CI +# +# When a shard has both "light" packages (run whole) and "heavy" package +# splits (run with -run regex), we need multiple gotestsum invocations. +# The Makefile's test-server target only supports a single invocation, +# so this script calls gotestsum directly. +# +# Each invocation produces its own JUnit XML and JSON log files, which +# are merged at the end into the standard report.xml and gotestsum.json +# that the CI pipeline expects. +# +# Input files (in working directory, written by shard-split.js): +# shard-te-packages.txt — space-separated TE packages +# shard-ee-packages.txt — space-separated EE packages +# shard-heavy-runs.txt — one line per heavy run: "pkg REGEX" +# +# Environment variables (set by CI): +# RACE_MODE — "-race" on master, empty on PRs +# ENABLE_COVERAGE — "true" to enable coverage profiling + +GOBIN="$(pwd)/bin" + +# Set up build prerequisites (go.work, gotestsum, go versions) +# These are normally done by make test-server-pre. +make setup-go-work gotestsum golang-versions + +GOFLAGS_BASE="-buildvcs=false -timeout=90m" +RACE_FLAG="${RACE_MODE:-}" + +RUN_IDX=0 +FAILURES=0 + +# run_gotestsum PACKAGES [RUN_REGEX] +# $1 = space-separated package list +# $2 = optional -run regex (passed directly to go test) +run_gotestsum() { + local junitfile="report-${RUN_IDX}.xml" + local jsonfile="gotestsum-${RUN_IDX}.json" + local run_flag="" + if [[ -n "${2:-}" ]]; then run_flag="-run $2"; fi + + local coverage_flag="" + if [[ "${ENABLE_COVERAGE:-false}" == "true" ]]; then + coverage_flag="-coverprofile=cover-${RUN_IDX}.out -covermode=atomic" + fi + + RUN_IDX=$((RUN_IDX + 1)) + + GOTESTSUM_JUNITFILE="$junitfile" GOTESTSUM_JSONFILE="$jsonfile" \ + "$GOBIN/gotestsum" --format "${GOTESTSUM_FORMAT:-testname}" --rerun-fails=3 --packages="$1" \ + -- $GOFLAGS_BASE $RACE_FLAG $coverage_flag $run_flag \ + || FAILURES=$((FAILURES + 1)) +} + +# ── Read shard assignments ── +SHARD_TE="" +SHARD_EE="" +HEAVY_RUNS="" + +if [[ -f shard-te-packages.txt ]]; then + SHARD_TE=$(cat shard-te-packages.txt) +fi +if [[ -f shard-ee-packages.txt ]]; then + SHARD_EE=$(cat shard-ee-packages.txt) +fi +if [[ -f shard-heavy-runs.txt && -s shard-heavy-runs.txt ]]; then + HEAVY_RUNS=$(cat shard-heavy-runs.txt) +fi + +# ── Run light packages (single invocation, no -run filter) ── +ALL_LIGHT="${SHARD_TE} ${SHARD_EE}" +ALL_LIGHT="${ALL_LIGHT## }" +ALL_LIGHT="${ALL_LIGHT%% }" +if [[ -n "$ALL_LIGHT" ]]; then + LIGHT_COUNT=$(echo "$ALL_LIGHT" | wc -w) + echo "Running $LIGHT_COUNT light packages..." + run_gotestsum "$ALL_LIGHT" +fi + +# ── Run heavy package splits (one invocation per package subset) ── +if [[ -n "$HEAVY_RUNS" ]]; then + while IFS= read -r line; do + [[ -z "$line" ]] && continue + PKG="${line%% *}" + REGEX="${line#* }" + SHORT_PKG="${PKG##*/}" + TEST_COUNT=$(echo "$REGEX" | tr '|' '\n' | wc -l) + echo "Running $TEST_COUNT tests from $SHORT_PKG..." + run_gotestsum "$PKG" "$REGEX" + done <<< "$HEAVY_RUNS" +fi + +# ── Merge results from all runs ── +echo "Merging results from $RUN_IDX gotestsum runs..." + +if ls report-*.xml 1>/dev/null 2>&1; then + # Simple XML concatenation — the merge job uses junit-report-merger for proper merging + head -1 report-0.xml > report.xml + echo "" >> report.xml + for f in report-*.xml; do + grep -v "> report.xml || true + done + echo "" >> report.xml +fi + +cat gotestsum-*.json > gotestsum.json 2>/dev/null || true + +if [[ $FAILURES -gt 0 ]]; then + echo "Shard complete: $RUN_IDX gotestsum runs, $FAILURES failed" + exit 1 +fi + +echo "Shard complete: $RUN_IDX gotestsum runs, all passed" diff --git a/server/scripts/shard-split.js b/server/scripts/shard-split.js new file mode 100644 index 0000000000..12005f7691 --- /dev/null +++ b/server/scripts/shard-split.js @@ -0,0 +1,242 @@ +#!/usr/bin/env node +/** + * shard-split.js — Test shard assignment solver + * + * Splits Go test packages across N parallel CI runners using timing data + * from previous runs. Uses a two-tier strategy: + * + * 1. "Light" packages (< HEAVY_MS total runtime): assigned whole to a shard + * 2. "Heavy" packages (>= HEAVY_MS): individual tests distributed across + * shards using -run regex filters + * + * Timing data sources (in priority order): + * - gotestsum.json (JSONL): per-test elapsed times from previous run + * - prev-report.xml (JUnit XML): package-level timing (fallback) + * - Round-robin: when no timing data exists at all + * + * Assignment algorithm: greedy bin-packing (sort by duration desc, assign + * each item to the shard with lowest current load). Simple and effective + * for our distribution where 2 packages dominate 84% of runtime. + * + * Environment variables: + * SHARD_INDEX — this runner's index (0-based) + * SHARD_TOTAL — total number of shards + * + * Input files (in working directory): + * all-packages.txt — newline-separated list of all test packages + * prev-gotestsum.json — (optional) JSONL timing data from previous run + * prev-report.xml — (optional) JUnit XML from previous run + * + * Output files (in working directory): + * shard-te-packages.txt — space-separated TE packages for this shard + * shard-ee-packages.txt — space-separated EE packages for this shard + * shard-heavy-runs.txt — heavy package runs, one per line: "pkg REGEX" + */ + +const fs = require("node:fs"); +const { execSync } = require("node:child_process"); + +const SHARD_INDEX = parseInt(process.env.SHARD_INDEX); +const SHARD_TOTAL = parseInt(process.env.SHARD_TOTAL); +const HEAVY_MS = 300000; // 5 min: packages above this get test-level splitting +// Only api4 (~38 min) and app (~15 min) exceed this threshold. +// Packages like sqlstore (~3 min) stay whole to preserve test isolation — +// their integrity tests scan the entire database and break if split across +// shards where other tests leave data behind. + +if (isNaN(SHARD_INDEX) || isNaN(SHARD_TOTAL) || SHARD_TOTAL < 1) { + console.error("ERROR: SHARD_INDEX and SHARD_TOTAL must be set"); + process.exit(1); +} + +const allPkgs = fs.readFileSync("all-packages.txt", "utf8").trim().split("\n").filter(Boolean); +if (allPkgs.length === 0) { + console.error("WARNING: No test packages found in all-packages.txt"); + process.exit(0); +} + +const pkgTimes = {}; +const testTimes = {}; // "pkg::TestName" -> ms + +// ── Parse gotestsum.json (JSONL) for per-test timing ── +// Each line is a JSON event; we want "pass" events with Elapsed times. +if (fs.existsSync("prev-gotestsum.json")) { + console.log("::group::Parsing gotestsum.json timing data"); + const lines = fs.readFileSync("prev-gotestsum.json", "utf8").split("\n"); + for (const line of lines) { + if (!line.includes('"pass"')) continue; + try { + const d = JSON.parse(line); + if (!d.Test || !d.Package) continue; + const elapsed = Math.round((d.Elapsed || 0) * 1000); + // Aggregate package time from test pass events + pkgTimes[d.Package] = (pkgTimes[d.Package] || 0) + elapsed; + // Top-level test name (use max elapsed for parent vs subtests) + const top = d.Test.split("/")[0]; + const key = d.Package + "::" + top; + testTimes[key] = Math.max(testTimes[key] || 0, elapsed); + } catch (e) { + // Skip malformed lines + } + } + console.log( + `gotestsum.json: ${Object.keys(pkgTimes).length} packages, ${Object.keys(testTimes).length} tests` + ); + console.log("::endgroup::"); +} + +// ── Fallback: parse JUnit XML for package-level timing ── +if (Object.keys(pkgTimes).length === 0 && fs.existsSync("prev-report.xml")) { + console.log("::group::Parsing JUnit XML timing data (fallback)"); + const xml = fs.readFileSync("prev-report.xml", "utf8"); + for (const m of xml.matchAll(/]*>/g)) { + const name = m[0].match(/name="([^"]+)"/)?.[1]; + const time = m[0].match(/\btime="([^"]+)"/)?.[1]; + if (name && time) { + pkgTimes[name] = (pkgTimes[name] || 0) + Math.round(parseFloat(time) * 1000); + } + } + console.log(`JUnit XML: ${Object.keys(pkgTimes).length} packages (no per-test data)`); + console.log("::endgroup::"); +} + +const hasTimingData = Object.keys(pkgTimes).length > 0; +const hasTestTiming = Object.keys(testTimes).length > 0; + +// ── Identify heavy packages ── +// Only split at test level if we have per-test timing data +const heavyPkgs = new Set(); +if (hasTestTiming) { + for (const [pkg, ms] of Object.entries(pkgTimes)) { + if (ms > HEAVY_MS) heavyPkgs.add(pkg); + } +} +if (heavyPkgs.size > 0) { + console.log("Heavy packages (test-level splitting):"); + for (const p of heavyPkgs) { + console.log(` ${(pkgTimes[p] / 1000).toFixed(0)}s ${p.split("/").pop()}`); + } +} + +// ── Build work items ── +// Each item is either a whole package ("P") or a single test from a heavy package ("T") +const items = []; +for (const pkg of allPkgs) { + if (heavyPkgs.has(pkg)) { + // Split into individual test items + const tests = Object.entries(testTimes) + .filter(([k]) => k.startsWith(pkg + "::")) + .map(([k, ms]) => ({ ms, type: "T", pkg, test: k.split("::")[1] })); + if (tests.length > 0) { + items.push(...tests); + } else { + // Shouldn't happen, but fall back to whole package + items.push({ ms: pkgTimes[pkg] || 1, type: "P", pkg }); + } + } else { + items.push({ ms: pkgTimes[pkg] || 1, type: "P", pkg }); + } +} +// ── Discover new/renamed tests in heavy packages ── +// Tests not in the timing cache won't appear in any shard's -run regex, +// silently skipping them. Discover current test names at runtime and +// assign any cache-missing tests to the least-loaded shard. +if (heavyPkgs.size > 0) { + console.log("::group::Discovering new tests in heavy packages"); + for (const pkg of heavyPkgs) { + const cachedTests = new Set( + Object.keys(testTimes) + .filter((k) => k.startsWith(pkg + "::")) + .map((k) => k.split("::")[1]) + ); + try { + const out = execSync(`go test -list '.*' ${pkg} 2>/dev/null`, { + encoding: "utf8", + timeout: 60000, + }); + const currentTests = out + .split("\n") + .map((l) => l.trim()) + .filter((l) => /^Test[A-Z]/.test(l)); + let newCount = 0; + for (const t of currentTests) { + if (!cachedTests.has(t)) { + // Assign a small default duration so it gets picked up + items.push({ ms: 1000, type: "T", pkg, test: t }); + newCount++; + } + } + if (newCount > 0) { + console.log(` ${pkg.split("/").pop()}: ${newCount} new test(s) not in cache`); + } + } catch (e) { + console.log(` ${pkg.split("/").pop()}: go test -list failed, skipping discovery`); + } + } + console.log("::endgroup::"); +} + +// Sort descending by duration for greedy bin-packing +items.sort((a, b) => b.ms - a.ms); + +// ── Greedy bin-packing assignment ── +const shards = Array.from({ length: SHARD_TOTAL }, () => ({ + load: 0, + whole: [], + heavy: {}, +})); + +if (!hasTimingData) { + // Round-robin fallback when no timing data exists + console.log("No timing data — using round-robin"); + allPkgs.forEach((pkg, i) => { + shards[i % SHARD_TOTAL].whole.push(pkg); + }); +} else { + for (const item of items) { + // Find shard with minimum current load + const min = shards.reduce((m, s, i) => (s.load < shards[m].load ? i : m), 0); + shards[min].load += item.ms; + if (item.type === "P") { + shards[min].whole.push(item.pkg); + } else { + if (!shards[min].heavy[item.pkg]) shards[min].heavy[item.pkg] = []; + shards[min].heavy[item.pkg].push(item.test); + } + } +} + +// ── Report shard assignments ── +console.log("::group::Shard assignment"); +for (let i = 0; i < SHARD_TOTAL; i++) { + const s = shards[i]; + const hRuns = Object.keys(s.heavy).length; + const hTests = Object.values(s.heavy).reduce((n, a) => n + a.length, 0); + const marker = i === SHARD_INDEX ? " ← THIS SHARD" : ""; + console.log( + `Shard ${i}: ${(s.load / 1000).toFixed(1)}s | ${s.whole.length} pkgs` + + (hRuns > 0 ? `, ${hRuns} heavy splits (${hTests} tests)` : "") + + marker + ); +} +console.log("::endgroup::"); + +// ── Write output for this shard ── +const myShard = shards[SHARD_INDEX]; +const te = myShard.whole.filter((p) => !p.includes("/enterprise/")).join(" "); +const ee = myShard.whole.filter((p) => p.includes("/enterprise/")).join(" "); + +fs.writeFileSync("shard-te-packages.txt", te); +fs.writeFileSync("shard-ee-packages.txt", ee); + +// Heavy package runs: one line per run as "pkg REGEX" +const heavyRuns = Object.entries(myShard.heavy).map(([pkg, tests]) => { + const regex = tests.map((t) => "^" + t + "$").join("|"); + return pkg + " " + regex; +}); +fs.writeFileSync("shard-heavy-runs.txt", heavyRuns.join("\n")); + +console.log( + `Light packages: ${myShard.whole.length} (${te.split(" ").filter(Boolean).length} TE, ${ee.split(" ").filter(Boolean).length} EE)` +); +console.log(`Heavy package runs: ${heavyRuns.length}`); diff --git a/server/scripts/shard-split.test.js b/server/scripts/shard-split.test.js new file mode 100644 index 0000000000..1e558f873f --- /dev/null +++ b/server/scripts/shard-split.test.js @@ -0,0 +1,333 @@ +const { describe, it, beforeEach, afterEach } = require("node:test"); +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const path = require("node:path"); +const { execFileSync } = require("node:child_process"); +const os = require("node:os"); + +const SCRIPT = path.join(__dirname, "shard-split.js"); +const TESTDATA = path.join(__dirname, "testdata"); + +/** + * Helper: run shard-split.js in a temp directory with given inputs. + * Returns the output files and stdout. + */ +function runSolver({ packages, shardIndex, shardTotal, gotestsumJson, prevReportXml }) { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "shard-test-")); + try { + fs.writeFileSync(path.join(tmpDir, "all-packages.txt"), packages.join("\n")); + + if (gotestsumJson) { + fs.writeFileSync(path.join(tmpDir, "prev-gotestsum.json"), gotestsumJson); + } + if (prevReportXml) { + fs.writeFileSync(path.join(tmpDir, "prev-report.xml"), prevReportXml); + } + + const stdout = execFileSync("node", [SCRIPT], { + cwd: tmpDir, + env: { + ...process.env, + SHARD_INDEX: String(shardIndex), + SHARD_TOTAL: String(shardTotal), + }, + encoding: "utf8", + }); + + const te = fs.readFileSync(path.join(tmpDir, "shard-te-packages.txt"), "utf8"); + const ee = fs.readFileSync(path.join(tmpDir, "shard-ee-packages.txt"), "utf8"); + const heavy = fs.readFileSync(path.join(tmpDir, "shard-heavy-runs.txt"), "utf8"); + + return { te, ee, heavy, stdout }; + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +} + +describe("shard-split.js", () => { + describe("round-robin fallback (no timing data)", () => { + it("distributes packages evenly across shards", () => { + const packages = [ + "github.com/mattermost/mattermost/server/v8/channels/api4", + "github.com/mattermost/mattermost/server/v8/channels/app", + "github.com/mattermost/mattermost/server/v8/channels/store/sqlstore", + "github.com/mattermost/mattermost/server/v8/config", + ]; + + // Collect assignments from all shards + const allTe = []; + for (let i = 0; i < 2; i++) { + const result = runSolver({ packages, shardIndex: i, shardTotal: 2 }); + allTe.push(...result.te.split(" ").filter(Boolean)); + } + + // All packages should be assigned exactly once + assert.equal(allTe.sort().join("\n"), packages.sort().join("\n")); + }); + + it("uses round-robin when no timing files exist", () => { + const packages = ["pkg/a", "pkg/b", "pkg/c", "pkg/d", "pkg/e"]; + const r0 = runSolver({ packages, shardIndex: 0, shardTotal: 2 }); + const r1 = runSolver({ packages, shardIndex: 1, shardTotal: 2 }); + + assert.ok(r0.stdout.includes("round-robin"), "Should mention round-robin in output"); + // No heavy runs + assert.equal(r0.heavy.trim(), ""); + assert.equal(r1.heavy.trim(), ""); + }); + }); + + describe("timing-based balancing", () => { + it("balances shards using gotestsum.json timing data", () => { + const gotestsumJson = fs.readFileSync( + path.join(TESTDATA, "sample-gotestsum.json"), + "utf8" + ); + const packages = [ + "github.com/mattermost/mattermost/server/v8/channels/api4", + "github.com/mattermost/mattermost/server/v8/channels/app", + "github.com/mattermost/mattermost/server/v8/channels/store/sqlstore", + "github.com/mattermost/mattermost/server/v8/config", + "github.com/mattermost/mattermost/server/v8/enterprise/elasticsearch", + "github.com/mattermost/mattermost/server/v8/enterprise/compliance", + "github.com/mattermost/mattermost/server/public/model", + ]; + + // Run for all 4 shards and check that loads are somewhat balanced + const loads = []; + const allAssigned = new Set(); + + for (let i = 0; i < 4; i++) { + const result = runSolver({ + packages, + shardIndex: i, + shardTotal: 4, + gotestsumJson, + }); + + // Track all assigned packages and tests + const tePkgs = result.te.split(" ").filter(Boolean); + const eePkgs = result.ee.split(" ").filter(Boolean); + tePkgs.forEach((p) => allAssigned.add(p)); + eePkgs.forEach((p) => allAssigned.add(p)); + + // Parse heavy runs + if (result.heavy.trim()) { + result.heavy + .trim() + .split("\n") + .forEach((line) => { + const pkg = line.split(" ")[0]; + allAssigned.add(pkg); + }); + } + } + + // Every package should be covered + for (const pkg of packages) { + assert.ok( + allAssigned.has(pkg), + `Package ${pkg} should be assigned to some shard` + ); + } + }); + + it("does not produce empty shards with sample data", () => { + const gotestsumJson = fs.readFileSync( + path.join(TESTDATA, "sample-gotestsum.json"), + "utf8" + ); + const packages = [ + "github.com/mattermost/mattermost/server/v8/channels/api4", + "github.com/mattermost/mattermost/server/v8/channels/app", + "github.com/mattermost/mattermost/server/v8/channels/store/sqlstore", + "github.com/mattermost/mattermost/server/v8/config", + ]; + + for (let i = 0; i < 4; i++) { + const result = runSolver({ + packages, + shardIndex: i, + shardTotal: 4, + gotestsumJson, + }); + const hasWork = + result.te.trim() !== "" || + result.ee.trim() !== "" || + result.heavy.trim() !== ""; + assert.ok(hasWork, `Shard ${i} should have some work assigned`); + } + }); + }); + + describe("heavy package splitting", () => { + it("splits packages over HEAVY_MS threshold into individual tests", () => { + // Create timing data where api4 is very heavy (> 300s = 300000ms) + const lines = []; + // api4: 6 tests totaling 452.2s (> 300s threshold) + for (const [test, elapsed] of [ + ["TestGetChannel", 145.2], + ["TestCreatePost", 98.1], + ["TestUpdateChannel", 72.5], + ["TestDeleteChannel", 58.3], + ["TestGetChannelMembers", 45.7], + ["TestSearchChannels", 32.4], + ]) { + lines.push( + JSON.stringify({ + Time: "2025-03-20T10:00:00Z", + Action: "pass", + Package: "github.com/mattermost/mattermost/server/v8/channels/api4", + Test: test, + Elapsed: elapsed, + }) + ); + } + // config: 2 tests totaling 8s (< 120s, stays whole) + for (const [test, elapsed] of [ + ["TestConfigStore", 5.0], + ["TestConfigMigrate", 3.0], + ]) { + lines.push( + JSON.stringify({ + Time: "2025-03-20T10:00:00Z", + Action: "pass", + Package: "github.com/mattermost/mattermost/server/v8/config", + Test: test, + Elapsed: elapsed, + }) + ); + } + + const gotestsumJson = lines.join("\n"); + const packages = [ + "github.com/mattermost/mattermost/server/v8/channels/api4", + "github.com/mattermost/mattermost/server/v8/config", + ]; + + // With 2 shards, api4 tests should be split across shards + let heavyFound = false; + const allHeavyTests = []; + + for (let i = 0; i < 2; i++) { + const result = runSolver({ + packages, + shardIndex: i, + shardTotal: 2, + gotestsumJson, + }); + + if (result.heavy.trim()) { + heavyFound = true; + // Parse heavy runs to extract test names + for (const line of result.heavy.trim().split("\n")) { + const parts = line.split(" "); + assert.equal( + parts[0], + "github.com/mattermost/mattermost/server/v8/channels/api4", + "Heavy package should be api4" + ); + // Regex is like "^TestGetChannel$|^TestCreatePost$" + const tests = parts[1].split("|").map((r) => r.replace(/[\^$]/g, "")); + allHeavyTests.push(...tests); + } + } + } + + assert.ok(heavyFound, "Should have heavy package splits for api4"); + // All api4 tests should be distributed + const expectedTests = [ + "TestGetChannel", + "TestCreatePost", + "TestUpdateChannel", + "TestDeleteChannel", + "TestGetChannelMembers", + "TestSearchChannels", + ]; + assert.deepEqual( + allHeavyTests.sort(), + expectedTests.sort(), + "All api4 tests should be distributed across shards" + ); + }); + + it("keeps light packages whole even with timing data", () => { + const gotestsumJson = [ + '{"Action":"pass","Package":"pkg/light","Test":"TestA","Elapsed":5.0}', + '{"Action":"pass","Package":"pkg/light","Test":"TestB","Elapsed":3.0}', + ].join("\n"); + + const result = runSolver({ + packages: ["pkg/light"], + shardIndex: 0, + shardTotal: 2, + gotestsumJson, + }); + + // Light package should be assigned whole, not split + assert.equal(result.heavy.trim(), "", "Light package should not be in heavy runs"); + assert.ok( + result.te.includes("pkg/light"), + "Light package should be in TE packages" + ); + }); + }); + + describe("JUnit XML fallback", () => { + it("uses JUnit XML when gotestsum.json is missing", () => { + const prevReportXml = ` + + + + + + + + + +`; + + const result = runSolver({ + packages: ["pkg/fast", "pkg/slow"], + shardIndex: 0, + shardTotal: 2, + prevReportXml, + }); + + assert.ok( + result.stdout.includes("JUnit XML"), + "Should indicate using JUnit XML fallback" + ); + // No heavy splits with XML-only data (no per-test timing) + assert.equal(result.heavy.trim(), "", "Should not split packages without per-test timing"); + }); + }); + + describe("enterprise package separation", () => { + it("separates enterprise packages into EE output", () => { + const packages = [ + "github.com/mattermost/mattermost/server/v8/channels/app", + "github.com/mattermost/mattermost/server/v8/enterprise/compliance", + ]; + + const result = runSolver({ + packages, + shardIndex: 0, + shardTotal: 1, + }); + + assert.ok( + result.te.includes("channels/app"), + "TE should include non-enterprise packages" + ); + assert.ok( + result.ee.includes("enterprise/compliance"), + "EE should include enterprise packages" + ); + assert.ok( + !result.te.includes("enterprise"), + "TE should not include enterprise packages" + ); + }); + }); +}); diff --git a/server/scripts/testdata/sample-gotestsum.json b/server/scripts/testdata/sample-gotestsum.json new file mode 100644 index 0000000000..2b4c7d7862 --- /dev/null +++ b/server/scripts/testdata/sample-gotestsum.json @@ -0,0 +1,22 @@ +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/api4","Test":"TestGetChannel","Elapsed":45.2} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/api4","Test":"TestCreatePost","Elapsed":38.1} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/api4","Test":"TestUpdateChannel","Elapsed":22.5} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/api4","Test":"TestDeleteChannel","Elapsed":18.3} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/api4","Test":"TestGetChannelMembers","Elapsed":15.7} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/api4","Test":"TestSearchChannels","Elapsed":12.4} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/app","Test":"TestCreateUser","Elapsed":25.6} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/app","Test":"TestUpdateUser","Elapsed":20.3} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/app","Test":"TestDeleteUser","Elapsed":15.8} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/app","Test":"TestGetUser","Elapsed":10.2} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/app","Test":"TestSearchUsers","Elapsed":8.5} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/store/sqlstore","Test":"TestChannelStore","Elapsed":35.0} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/store/sqlstore","Test":"TestPostStore","Elapsed":28.0} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/channels/store/sqlstore","Test":"TestUserStore","Elapsed":22.0} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/config","Test":"TestConfigStore","Elapsed":5.0} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/config","Test":"TestConfigMigrate","Elapsed":3.0} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/enterprise/elasticsearch","Test":"TestSearchPosts","Elapsed":8.0} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/enterprise/elasticsearch","Test":"TestIndexPosts","Elapsed":6.0} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/v8/enterprise/compliance","Test":"TestExportCompliance","Elapsed":4.0} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/public/model","Test":"TestModelValidation","Elapsed":2.0} +{"Time":"2025-03-20T10:00:00Z","Action":"pass","Package":"github.com/mattermost/mattermost/server/public/model","Test":"TestModelSerialization","Elapsed":1.5} +{"Time":"2025-03-20T10:00:00Z","Action":"output","Package":"github.com/mattermost/mattermost/server/v8/channels/api4","Test":"TestGetChannel","Output":"--- PASS: TestGetChannel (45.20s)\n"}