Skip to content

backend/fix: implement withForkCounters in the cluster pipeline test monad - #1517

Merged
piyushKumar-1 merged 1 commit into
mainfrom
backend/fix/test-core-metrics-fork-counters
Sep 13, 2026
Merged

piyushKumar-1 merged 1 commit into
mainfrom
backend/fix/test-core-metrics-fork-counters

Conversation

@piyushKumar-1

@piyushKumar-1 piyushKumar-1 commented Sep 13, 2026 •

Copy link
Copy Markdown
Member

What

Adds withForkCounters _ _ action = action to the CoreMetrics instance of TestM in test/src/HedisClusterPipeline.hs.

Why

0a8170e9 added withForkCounters to the CoreMetrics class without a default. #1516 was based on the commit before it and added a CoreMetrics instance for the Redis cluster test monad that doesn't define it. Both merged cleanly on their own, but together mobility-core-tests fails with -Wmissing-methods under -Werror. Only the test suite is affected; the library is unchanged.

TestM's metrics are all no-ops, so withForkCounters just runs the action. No other CoreMetrics instance in the repo is missing the method.

Summary by CodeRabbit

  • Tests
    • Updated the test environment to support fork-counting metrics without changing test behavior.

…monad

0a8170e added withForkCounters to CoreMetrics without a default. #1516 was
based on the commit before it and added a CoreMetrics instance for the
Redis cluster test monad (TestM) that does not define it, so after both
merged, mobility-core-tests fails with -Wmissing-methods under -Werror.
TestM's metrics are no-ops, so withForkCounters just runs the action.
@coderabbitai

coderabbitai Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d7c87a85-ec95-4a0f-bf8f-c656c3632905

📥 Commits

Reviewing files that changed from the base of the PR and between ea37819 and 436fbda.

📒 Files selected for processing (1)
  • lib/mobility-core/test/src/HedisClusterPipeline.hs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The Metrics.CoreMetrics TestM instance now implements withForkCounters as a no-op. It returns the supplied action unchanged.

Changes

Test Metrics Support

Layer / File(s) Summary
No-op fork counter implementation
lib/mobility-core/test/src/HedisClusterPipeline.hs
The TestM metrics instance implements withForkCounters and returns the action unchanged.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 436fb

This test-only change completes the metrics stub without changing runtime behavior.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: implementing withForkCounters in the cluster pipeline test monad.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch backend/fix/test-core-metrics-fork-counters

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

A rabbit counts no forks today
The test path keeps its quiet way
Metrics pause, the action runs
No extra counters chase the buns
The stub now fits the meadow’s code

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

@piyushKumar-1
piyushKumar-1 merged commit d7123b1 into main Sep 13, 2026
2 checks passed
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.

1 participant