Skip to content

test: extract setUp in BT1226IfNotNilMutationsTest#2826

Merged
jamesc merged 3 commits into
mainfrom
test-quality/bt-extract-setup-if-not-nil-mut
Jul 4, 2026
Merged

test: extract setUp in BT1226IfNotNilMutationsTest#2826
jamesc merged 3 commits into
mainfrom
test-quality/bt-extract-setup-if-not-nil-mut

Conversation

@jamesc

@jamesc jamesc commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • All 6 test methods in BT1226IfNotNilMutationsTest opened with an identical a := IfNotNilMutActor spawn line, duplicating the fixture setup in every test.
  • Extract to a setUp method so the shared fixture is declared once. If the fixture class ever changes, there is one place to update instead of six.
  • Replace local variable a with self.a throughout. No assertion logic changed.

Test plan

  • just test-bunit passes: 250 files, 2666 tests, 0 failed (verified locally before and after the change).
  • Test count is unchanged — no tests were removed or added.

Smell

rg --count '// => _' stdlib/test/ and manual inspection surfaced this file as a repeated-setup candidate: six identical spawn lines across six test methods, no setUp, no tearDown needed.


Generated by Claude Code

… spawn

All 6 test methods opened with an identical `a := IfNotNilMutActor spawn`
line. Extracting to setUp makes the shared fixture explicit and means a
single edit if the fixture ever changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R6edQqFhmXN77NcRGZyN47
Comment thread stdlib/test/bt_1226_if_not_nil_mutations_test.bt
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown

✅ Claude Review — PASS

Code Review Summary

Overall: PR is clean — no blockers, no suggestions, no nits.

Changes Reviewed

The PR refactors bt_1226_if_not_nil_mutations_test.bt in two commits:

  1. Extract setUp (6eb9a587): Removes the repeated a := IfNotNilMutActor spawn at the top of each of the six test methods, replaces with a shared field: a = nil, setUp => self.a := IfNotNilMutActor spawn, and rewrites all a … references to self.a ….

  2. Add tearDown (1f7eda0e, this push): Adds tearDown => self.a stop so the actor is cleaned up after each test.

Correctness Assessment

  • Each test receives a fresh actor: setUp is called per-test in BUnit, so self.a is always a newly spawned IfNotNilMutActor with empty running state. No cross-test state bleed.
  • self.a := IfNotNilMutActor spawn in setUp is the same pattern used by ConditionalHofStateTest and actor_threading_constructs_test; the return value correctly threads the updated TestCase instance to each test method.
  • tearDown => self.a stop matches the bare-stop pattern used by ConditionalHofStateTest, actor_threading_constructs_test, and actor_self_send_collect_test. stop is documented as idempotent. IfNotNilMutActor has no custom initialize, so spawn cannot fail and the nil case is unreachable.
  • No tests stop the actor mid-method, so there is no double-stop scenario.
  • The test body logic is unchanged; only the actor lifecycle scaffolding is refactored.

claude added 2 commits July 4, 2026 08:07
Addresses review suggestion: every other stdlib test class using
self.a := in setUp declares the slot with a default. Without it, a
runner fallback to a bare Module:new() instance would crash with
{badkey, a} on first self.a access instead of a clean nil. Makes the
fixture slot explicit and any fallback a diagnosable failure.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015RFHaym8aCseDmmZ417jn2
Addresses Claude review suggestion: analogous actor-spawning tests
(conditional_hof_state_test, actor_threading_constructs_test,
future_auto_await_test) stop their fixture actor in tearDown. Adding it
here stops the spawned actor after each test instead of accumulating six
live processes across the suite. just test-bunit green (2666, 0 failed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015RFHaym8aCseDmmZ417jn2
@jamesc jamesc merged commit 5fa7b50 into main Jul 4, 2026
9 checks passed
@jamesc jamesc deleted the test-quality/bt-extract-setup-if-not-nil-mut branch July 4, 2026 08:59
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