Skip to content

Commit b641145

Browse files
ncodeclaude
andcommitted
fix: address PR review feedback
- Add defer ballot.releaseSession() to all integration tests for proper session cleanup - Simplify TestIntegration_TagPromotion to test promotion only (not the incomplete demotion scenario) - Fix Makefile test-integration target to run cleanup even on test failure 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent eaac16b commit b641145

File tree

2 files changed

+11
-18
lines changed

2 files changed

+11
-18
lines changed

Makefile

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@ test-coverage:
1414
go tool cover -html=coverage.out -o coverage.html
1515

1616
test-integration: integration-up
17-
CONSUL_HTTP_ADDR=http://localhost:8500 go test -v -tags=integration -race ./...
18-
$(MAKE) integration-down
17+
CONSUL_HTTP_ADDR=http://localhost:8500 go test -v -tags=integration -race ./... ; \
18+
status=$$? ; \
19+
$(MAKE) integration-down ; \
20+
exit $$status
1921

2022
integration-up:
2123
docker compose -f configs/integration/docker-compose.yaml up -d --wait

internal/ballot/ballot_integration_test.go

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ func TestIntegration_FullElectionCycle(t *testing.T) {
114114

115115
ballot, err := New(ctx, serviceID)
116116
require.NoError(t, err, "Failed to create Ballot instance")
117+
defer ballot.releaseSession()
117118

118119
// Run a single election cycle
119120
err = ballot.election()
@@ -160,6 +161,7 @@ func TestIntegration_LeaderFailover(t *testing.T) {
160161
setupViper(t, serviceID1, electionKey)
161162
ballot1, err := New(ctx, serviceID1)
162163
require.NoError(t, err)
164+
defer ballot1.releaseSession()
163165

164166
// Setup and create second ballot
165167
viper.Set(fmt.Sprintf("election.services.%s.id", serviceID2), serviceID2)
@@ -170,6 +172,7 @@ func TestIntegration_LeaderFailover(t *testing.T) {
170172
viper.Set(fmt.Sprintf("election.services.%s.lockDelay", serviceID2), "1s")
171173
ballot2, err := New(ctx, serviceID2)
172174
require.NoError(t, err)
175+
defer ballot2.releaseSession()
173176
defer viper.Reset()
174177

175178
// First ballot becomes leader
@@ -222,6 +225,7 @@ func TestIntegration_TagPromotion(t *testing.T) {
222225

223226
ballot, err := New(ctx, serviceID)
224227
require.NoError(t, err)
228+
defer ballot.releaseSession()
225229

226230
// Verify service doesn't have primary tag initially
227231
service, _, err := client.Agent().Service(serviceID, nil)
@@ -237,22 +241,6 @@ func TestIntegration_TagPromotion(t *testing.T) {
237241
service, _, err = client.Agent().Service(serviceID, nil)
238242
require.NoError(t, err)
239243
assert.Contains(t, service.Tags, testPrimaryTag, "Should have primary tag after becoming leader")
240-
241-
// Release session to trigger demotion
242-
err = ballot.releaseSession()
243-
require.NoError(t, err)
244-
245-
// Wait briefly for session release
246-
time.Sleep(500 * time.Millisecond)
247-
248-
// Run election again - should update status since session is gone
249-
err = ballot.election()
250-
require.NoError(t, err)
251-
252-
// Verify we're no longer leader (need another election cycle to update tags)
253-
// The leadership status depends on whether we re-acquired the lock
254-
// After releasing session, we should create a new one and potentially become leader again
255-
// For this test, we just verify the tag manipulation works
256244
}
257245

258246
func TestIntegration_HealthCheckFailure(t *testing.T) {
@@ -274,6 +262,7 @@ func TestIntegration_HealthCheckFailure(t *testing.T) {
274262

275263
ballot, err := New(ctx, serviceID)
276264
require.NoError(t, err)
265+
defer ballot.releaseSession()
277266

278267
// Become leader first
279268
err = ballot.election()
@@ -329,6 +318,7 @@ func TestIntegration_MultipleInstances(t *testing.T) {
329318
setupViper(t, services[i], electionKey)
330319
b, err := New(ctx, services[i])
331320
require.NoError(t, err)
321+
defer b.releaseSession()
332322
ballots[i] = b
333323
}
334324
defer cleanupKV(t, client, electionKey)
@@ -391,6 +381,7 @@ func TestIntegration_SessionRenewal(t *testing.T) {
391381

392382
ballot, err := New(ctx, serviceID)
393383
require.NoError(t, err)
384+
defer ballot.releaseSession()
394385

395386
// Become leader
396387
err = ballot.election()

0 commit comments

Comments
 (0)