Skip to content

fix: flush global state in queue's createPayloadCallbacks#394

Merged
albertcht merged 1 commit into
0.3from
hotfix/queue-payload-closure-testing
Jun 3, 2026
Merged

fix: flush global state in queue's createPayloadCallbacks#394
albertcht merged 1 commit into
0.3from
hotfix/queue-payload-closure-testing

Conversation

@albertcht
Copy link
Copy Markdown
Member

This PR fixes a potential memory leak in queue's createPayloadCallbacks state in testing.

@albertcht albertcht added the bug Something isn't working label Jun 3, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 3, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 26903e42-a762-47d2-bcbf-693029cd2404

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hotfix/queue-payload-closure-testing

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.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 3, 2026

Greptile Summary

Adds a single Queue::createPayloadUsing(null) call in TestCase::tearDown() to flush the static $createPayloadCallbacks array on Queue after each test, preventing registered payload callbacks from leaking into subsequent tests.

  • Static state cleanup: Queue::$createPayloadCallbacks is a static array; without this reset, any callback registered via Queue::createPayloadUsing() in one test persists into the next, potentially mutating job payloads unexpectedly.
  • Placement: The reset is placed inside the if ($this->app) guard alongside the other app-teardown calls; since setUp() always ensures $this->app is initialized before callbacks can be registered, this guard is always entered in normal test flow.

Confidence Score: 5/5

This PR is safe to merge — it adds a single, well-scoped cleanup call with no risk of side effects in production code.

The change resets a static array that only accumulates test-registered callbacks. The createPayloadUsing(null) API already exists and is the documented way to clear the array. The placement in tearDown() follows the same pattern as other static-state cleanup already present in the file (Carbon, Mockery). No production code path is touched.

No files require special attention.

Important Files Changed

Filename Overview
src/foundation/src/Testing/TestCase.php Adds Queue::createPayloadUsing(null) in tearDown() to reset the static $createPayloadCallbacks array between tests, preventing cross-test state bleed.

Reviews (1): Last reviewed commit: "fix: flush global state in queue's creat..." | Re-trigger Greptile

@albertcht albertcht merged commit 5b90f5c into 0.3 Jun 3, 2026
11 of 17 checks passed
@albertcht albertcht deleted the hotfix/queue-payload-closure-testing branch June 3, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant