Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Clean up custom Queue payload between tests #36271

Merged
merged 4 commits into from
Feb 15, 2021

Conversation

deleugpn
Copy link
Contributor

When executing tests with Data Providers, Queue::createPayloadUsing leaks between tests and accumulates callbacks from different environments, leading to unexpected behaviors.

This pull request adds a clean up to the TearDown method so that callbacks are not kept between tests.
The test is likely to fail until orchestral/testbench-core#54 is merged as it mimics the same behavior for tests executed from within the framework.

@crynobone
Copy link
Member

So it's not an issue if queue worker states persist between job (one worker may handle different jobs which might not related) and we only need to handle it between tests?

@deleugpn
Copy link
Contributor Author

deleugpn commented Feb 15, 2021

@crynobone The worker only bootstraps Laravel once and then can run as many job as necessary. The problem is that during tests, Laravel bootstraps and tears down once per test, but the tear down is not clearing the createPayloadUsing closures which leads to several closures being accumulated statically on the Queue::createPayloadUsing because the setUp is re-running all Service Providers.

I noticed this problem because the 1st test passes, but the 2nd fails. During the 2nd tests, there are 2 closures bound into the Queue::createPayloadUsing: 1 closure is the old container and 1 closure is the new container. The old container should have been cleared out and is not in a valid state anymore.

deleugpn and others added 2 commits February 15, 2021 15:02
@taylorotwell
Copy link
Member

Hey @deleugpn - we had to revert this due to some stuff in Telescope. An alternative might be calling Queue::createPayloadUsing(null) in the tearDown method of Laravel's base test case. We already reset some stuff there.

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.

3 participants