[bug] fix ut TestNodeGossip

[volgariver6]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #18196

What this PR does / why we need it:

fix ut TestNodeGossip

---

PR Type

Bug fix, Tests

---

Description

• Fixed the TestNodeGossip unit test by introducing dynamic port allocation to avoid port conflicts.
• Added a new function getRandomPort to generate random ports for service and cache addresses.
• Updated the test to ensure nodes join using dynamically assigned ports, improving test reliability.

---

Changes walkthrough 📝

	Relevant files
Tests	node_test.go Use dynamic port allocation in TestNodeGossip                        pkg/gossip/node_test.go Introduced a function getRandomPort to generate random ports. Updated TestNodeGossip to use random ports for service and cache addresses. Ensured nodes join using dynamically assigned ports. +19/-8

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Potential Race Condition The getRandomPort() function uses a new random seed each time it's called, which could potentially lead to duplicate port numbers if called in quick succession. Error Handling The code doesn't handle the case where getRandomPort() might return a port that's already in use, which could lead to test failures.

[codiumai-pr-agent-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Code Suggestions ✨

Category	Suggestion	Score
Enhancement	Use a more reliable method to find available ports dynamically Consider using a more robust method for generating random ports. The current implementation might lead to port conflicts if multiple tests are run simultaneously. Consider using a package like net to find available ports dynamically. pkg/gossip/node_test.go [28-31] -func getRandomPort() int { - rand.New(rand.NewSource(time.Now().UnixNano())) - return rand.Intn(65535-1024) + 1024 +func getAvailablePort() (int, error) { + addr, err := net.ResolveTCPAddr("tcp", "localhost:0") + if err != nil { + return 0, err + } + l, err := net.ListenTCP("tcp", addr) + if err != nil { + return 0, err + } + defer l.Close() + return l.Addr().(*net.TCPAddr).Port, nil } Apply this suggestion Suggestion importance[1-10]: 9 Why: The suggestion to use the net package for finding available ports is valid and addresses potential port conflicts when running multiple tests simultaneously. This is a significant improvement over the current random port generation method.	9
Error handling	Add error handling for port allocation in the test setup Consider adding error handling for the getRandomPort() function calls. If port allocation fails, the test should handle it gracefully. pkg/gossip/node_test.go [36-37] -svcPort1 := getRandomPort() -cachePort1 := getRandomPort() +svcPort1, err := getAvailablePort() +if err != nil { + t.Fatalf("Failed to get available port: %v", err) +} +cachePort1, err := getAvailablePort() +if err != nil { + t.Fatalf("Failed to get available port: %v", err) +} Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding error handling for port allocation is important for robustness, ensuring that the test fails gracefully if a port cannot be allocated. This is a valuable enhancement to the test setup.	8
Possible issue	Ensure that service and cache ports are different to avoid conflicts Consider adding a check to ensure that the randomly generated ports for service and cache are different to avoid potential conflicts. pkg/gossip/node_test.go [36-37] -svcPort1 := getRandomPort() -cachePort1 := getRandomPort() +svcPort1, err := getAvailablePort() +if err != nil { + t.Fatalf("Failed to get available port: %v", err) +} +cachePort1, err := getAvailablePort() +if err != nil { + t.Fatalf("Failed to get available port: %v", err) +} +if svcPort1 == cachePort1 { + t.Fatalf("Service and cache ports are the same: %d", svcPort1) +} Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion to check for port conflicts between service and cache ports is useful for preventing potential issues, although the likelihood of conflict is low with random generation. It still adds an extra layer of safety.	7
Best practice	Add a short delay after node creation to ensure proper initialization before joining Consider adding a small delay or retry mechanism after creating nodes to ensure they are fully initialized before attempting to join them together. pkg/gossip/node_test.go [74-79] err = n2.Create() assert.NoError(t, err) defer func() { assert.NoError(t, n2.Leave(time.Millisecond*300)) }() +time.Sleep(time.Millisecond * 100) // Allow nodes to initialize err = n2.Join([]string{fmt.Sprintf("127.0.0.1:%d", svcPort1)}) Apply this suggestion Suggestion importance[1-10]: 6 Why: Introducing a delay to ensure nodes are fully initialized before joining is a reasonable best practice, though it may not be strictly necessary if the node creation process is already reliable.	6

💡 Need additional feedback ? start a PR chat