Skip to content

test(controller): seed the DataVolume informer for controller-runtime 0.25 - #4729

Merged
IvanHunters merged 1 commit into
mainfrom
fix/controller-tests-fake-informer
Oct 2, 2026
Merged

test(controller): seed the DataVolume informer for controller-runtime 0.25#4729
IvanHunters merged 1 commit into
mainfrom
fix/controller-tests-fake-informer

Conversation

@lexfrei

@lexfrei Aleksei Sviridkin (lexfrei) commented Oct 2, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

What this PR does

main does not build internal/controller tests, so Unit & controller tests is red on every open PR. #4380 added DataVolume watch tests that set FakeInformer.Synced and lock SyncedLock. #4473 then moved to controller-runtime v0.25.1, where FakeInformer keeps its sync state in a private channel and exposes only Synced() and HasSynced(). Each PR was green on its own base.

The tests now seed the DataVolume informer into FakeInformers.InformersByGVK and call Synced() when they want it synced. Seeding is needed because FakeInformers builds every informer it creates itself already synced, so there is no way to get an unsynced one from it otherwise. The unsynced cases still assert that no reader is installed, so they would fail if the informer came up synced.

go vet ./... is clean and go test ./internal/controller/ passes.

Screenshots

Not a UI change.

Downstream repositories

Release note

NONE

Summary by CodeRabbit

  • Tests
    • Improved automated checks for workload monitoring across different data synchronization states. These changes strengthen validation of watch behavior without changing functionality visible to users.

@lexfrei
test(controller): seed the DataVolume informer for controller-runtime…
2eeaa59 
… 0.25

controller-runtime 0.25 made FakeInformer's sync state a private
channel, so the DataVolume watch tests no longer compile: they toggled
the exported Synced field and its lock. FakeInformers also builds every
informer it creates already synced, so an unsynced informer has to be
seeded into InformersByGVK and marked synced with Synced() later.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
@github-actions github-actions Bot added size/S This PR changes 10-29 lines, ignoring generated files area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review labels Oct 2, 2026
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 6308ea03-711c-4003-88e3-54b3e09de6b9

📥 Commits

Reviewing files that changed from the base of the PR and between bf03c84 and 2eeaa59.

📒 Files selected for processing (1)
  • internal/controller/workloadmonitor_controller_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The workload monitor tests now use a helper to create fake DataVolume informers and configure their sync state. Three watch tests use this helper instead of setting informer sync fields directly.

Changes

Informer test setup

Layer / File(s) Summary
Configure and use fake informers
internal/controller/workloadmonitor_controller_test.go		 Adds dataVolumeInformers(synced) and updates watch tests to use the helper and its Synced() method to configure informer sync state.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~4 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 2eeaa

The test fixture changes preserve the intended watch scenarios, so no actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: updating controller tests to seed the DataVolume informer for controller-runtime 0.25.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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.

@lexfrei Aleksei Sviridkin (lexfrei) mentioned this pull request Oct 2, 2026
Merged
1 of 12 tasks
@IvanHunters
IvanHunters enabled auto-merge October 2, 2026 20:25
@IvanHunters
IvanHunters merged commit 309875a into main Oct 2, 2026
56 checks passed
@IvanHunters
IvanHunters deleted the fix/controller-tests-fake-informer branch October 2, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@IvanHunters IvanHunters IvanHunters approved these changes
@kvaps Andrei Kvapil (kvaps) Awaiting requested review from kvaps kvaps is a code owner
@lllamnyp Timofei Larkin (lllamnyp) Awaiting requested review from lllamnyp lllamnyp is a code owner
@myasnikovdaniil myasnikovdaniil Awaiting requested review from myasnikovdaniil myasnikovdaniil is a code owner
@sircthulhu Kirill Ilin (sircthulhu) Awaiting requested review from sircthulhu sircthulhu is a code owner

Assignees

No one assigned

Labels

area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review size/S This PR changes 10-29 lines, ignoring generated files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lexfrei @IvanHunters