Skip to content

[client] Fix flaky TestUpdateOldManagementURL in CI#5703

Merged
lixmal merged 1 commit intomainfrom
fix-flaky-management-url-test
Mar 31, 2026
Merged

[client] Fix flaky TestUpdateOldManagementURL in CI#5703
lixmal merged 1 commit intomainfrom
fix-flaky-management-url-test

Conversation

@lixmal
Copy link
Copy Markdown
Collaborator

@lixmal lixmal commented Mar 26, 2026

Describe your changes

  • Mock the management gRPC client in TestUpdateOldManagementURL so it no longer makes real network calls to api.netbird.io, which times out in CI
  • Replace ModTime-based file change detection with content comparison, since the mock completes fast enough that writes can land within the same filesystem timestamp granularity

Issue ticket number and link

Fixes flaky Client / Unit CI failures on linux-amd64, linux-386, and FreeBSD runners (observed in #5697).

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Test-only change, no user-facing behavior affected.

Summary by CodeRabbit

  • Tests
    • Improved test infrastructure and assertions for management configuration updates.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1b6dc57e-5fe1-4ea3-a26c-54a06c1b944d

📥 Commits

Reviewing files that changed from the base of the PR and between 9aaa05e and b8191e8.

📒 Files selected for processing (2)
  • client/internal/profilemanager/config.go
  • client/internal/profilemanager/config_test.go

📝 Walkthrough

Walkthrough

Introduced a mgmProber interface and newMgmProber factory variable to enable management client overriding in tests. Updated UpdateOldManagementURL to use the factory instead of calling mgm.NewClient directly. Modified test assertions to compare file contents rather than modification times.

Changes

Cohort / File(s) Summary
Management Client Testability
client/internal/profilemanager/config.go
Added mgmProber interface and newMgmProber factory function to enable dependency injection. Refactored UpdateOldManagementURL to instantiate management client via factory. Adjusted error handling to use local scope for close operations.
Test Infrastructure Updates
client/internal/profilemanager/config_test.go
Implemented mockMgmProber test double with GetServerPublicKey() and Close() methods. Updated TestUpdateOldManagementURL to override factory function with mock. Changed assertions from comparing file modification times to comparing full serialized file contents.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • pappz

Poem

🐰 A factory springs forth, so clever and spry,
With mocks in the tests, we pass them on by,
Content now compared from start until end,
Testability blooms—a developer's best friend! 🌱

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: fixing a flaky test in CI by mocking the management client.
Description check ✅ Passed The description is comprehensive, addressing all major template sections: changes explained, issue reference provided, checklist completed, and documentation status clarified.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-flaky-management-url-test

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

@lixmal lixmal merged commit 6553ce4 into main Mar 31, 2026
42 checks passed
@lixmal lixmal deleted the fix-flaky-management-url-test branch March 31, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants