Skip to content

test: verify transactionLevel() reflects the active transaction insid… - #3572

Open
zigzagdev wants to merge 1 commit into
mongodb:5.xfrom
zigzagdev:fix/transaction-level-inside-closure
Open

test: verify transactionLevel() reflects the active transaction insid…#3572
zigzagdev wants to merge 1 commit into
mongodb:5.xfrom
zigzagdev:fix/transaction-level-inside-closure

Conversation

@zigzagdev

@zigzagdev zigzagdev commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Motivation

PHPLARA-29 / GH-2851

Reported that Connection::transactionLevel() returned 0 from inside the closure passed to transaction(), because the old implementation never set $this->transactions before invoking the callback.
And transaction() now calls handleInitialTransactionState() (which sets $this->transactions = 1) before invoking the callback, so this is already fixed as a side effect of later work on transaction event handling.

No production code change is needed.

What I have done

  • Add testTransactionLevelInsideClosure() to tests/TransactionTest.php to lock in the current (correct) behaviour and close out the ticket.

Checklist

  • Add tests and ensure they pass

…e transaction() closure

PHPLARA-29 / mongodbGH-2851 reported that Connection::transactionLevel()
returned 0 from inside the closure passed to transaction(), because
the old implementation never set $this->transactions before invoking
the callback.

transaction() now calls handleInitialTransactionState() (which sets
$this->transactions = 1) before invoking the callback, so this is
already fixed. Add a regression test to lock in the behavior and
close out the ticket.
@zigzagdev
zigzagdev requested a review from a team as a code owner August 26, 2026 14:02
@zigzagdev
zigzagdev requested a review from GromNaN August 26, 2026 14:02
@zigzagdev zigzagdev changed the title test: verify transactionLevel() reflects the active transaction insid… test: verify transactionLevel() reflects the active transaction insid… Aug 26, 2026
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