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

DB::afterCommit() callbacks and after commit jobs are incorrectly executed in the transaction in tests #50066

Closed
oprypkhantc opened this issue Feb 13, 2024 · 6 comments · Fixed by #50068

Comments

@oprypkhantc
Copy link
Contributor

Laravel Version

10.43.0

PHP Version

8.2.x

Database Driver & Version

No response

Description

Hey.

DB::afterCommit() and subsequently "after commit" jobs (and notifications, listeners, events etc) are incorrectly immediately executed when inside of a transaction in a PHPUnit test that uses DatabaseTransactions if the test has committed a transaction before:

class SomeTest extends TestCase {
	use DatabaseTransactions;

	public function testSomething(): void
	{
		DB::transaction(function () {});

		DB::transaction(function () {
			// This line should only execute after this closure
			DB::afterCommit(fn () => dd(123));
			
			// This line should execute first, before the line above
			dd(456);
		});
	}
}

which prints 123.

Steps To Reproduce

  1. Copy the test case above into a Laravel ^10.34 application and run it

Actual result is 123
Expected result is 456

@driesvints
Copy link
Member

Hey there,

Can you first please try one of the support channels below? If you can actually identify this as a bug, feel free to open up a new issue with a link to the original one and we'll gladly help you out.

Thanks!

@oprypkhantc
Copy link
Contributor Author

oprypkhantc commented Feb 13, 2024

@driesvints Why did you close this? I don't need any help or support. It's an issue with Laravel's DatabaseTransactionsManager implementation.

I can re-open the same issue but I don't think that's actually gonna help either us or Laravel.

@driesvints
Copy link
Member

Please try a support channel first, verify the problem with confirmation of others, then re-open a new issue with links to the support channel you tried. Thanks

@oprypkhantc
Copy link
Contributor Author

@driesvints I don't see how a community can confirm an edge-case bug in a feature that's barely used by anyone 🫠 Please don't force me to waste my time on those platforms; this is an issue tracker and I'm reporting an issue, just like others I've reported since 2020.

This issue is easily confirmed by removing the use DatabaseTransactions; from the test case. If you remove the trait use, you'll get the "expected" result I mentioned in the issue.

@driesvints
Copy link
Member

I'll re-open this and someone who has time can help you out.

Copy link

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants