Skip to content

fix(subscriptions): avoid races in teardown assertions - #80

Merged
yordis merged 1 commit into
mainfrom
yordis/fix-subscriptions-test-race
Apr 15, 2026
Merged

yordis merged 1 commit into
mainfrom
yordis/fix-subscriptions-test-race

Conversation

@yordis

@yordis yordis commented Apr 15, 2026

Copy link
Copy Markdown
Member
  • keep the subscriptions suite focused on registry behavior instead of monitor timing
  • remove a source of non-determinism from the full test run so failures reflect real regressions

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@cursor

cursor Bot commented Apr 15, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Test-only change that replaces manual monitoring with spawn_monitor to avoid racey :DOWN matching; no production code paths are modified.

Overview
Updates the subscriptions registry test to use spawn_monitor/1 instead of spawn_link plus Process.monitor/1, and tightens the :DOWN assertion to match the expected pid.

This reduces nondeterminism in the teardown timing so failures more reliably reflect real subscription registry regressions.

Reviewed by Cursor Bugbot for commit 4c7df9d. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Apr 15, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e6ceee67-b912-4043-bb88-fbfc700c1236

📥 Commits

Reviewing files that changed from the base of the PR and between 2b0a739 and 4c7df9d.

📒 Files selected for processing (1)
  • test/subscriptions/subscriptions_test.exs

Walkthrough

A test case is refactored to use spawn_monitor/1 instead of separate spawn_link/1 and Process.monitor/pid calls. The corresponding :DOWN message assertion is updated to pattern-match against both the monitor reference and process PID.

Changes

Cohort / File(s) Summary
Process Monitoring Refactor
test/subscriptions/subscriptions_test.exs
Simplifies process spawning and monitoring by replacing spawn_link/1 + Process.monitor/pid with a single spawn_monitor/1 call. The :DOWN assertion updated to pattern-match on both monitor reference and PID.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 A monitor spawned, now sleek and tight,
One call replaces two—what a delight!
The reference and PID dance in pattern's grace,
Process flows cleaner, no messy trace.
Hop, hop, refactor—the test's new pace! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing race conditions in teardown assertions by switching from spawn_link+monitor to spawn_monitor.
Description check ✅ Passed The description is directly related to the changeset, explaining the intent to focus on registry behavior and remove non-determinism from test timing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 yordis/fix-subscriptions-test-race

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.

@yordis
yordis merged commit 91a612f into main Apr 15, 2026
5 checks passed
@yordis
yordis deleted the yordis/fix-subscriptions-test-race branch April 15, 2026 04:31
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