Add commitTransaction command tests

[alinaliBQ]

This change adds tests for the $commitTransaction command operator.

Add command operator tests for $commitTransaction. Tests database $commitTransaction behavior, output collection, syntax, and expected errors.

• Add compatibility test for commitTransaction #411

[documentdb-triage-tool]

🤖 Auto-triaged by documentdb-triage-tool.

Applied: compatibility test, enhancement
Project fields suggested: Component test-coverage · Priority P2 · Effort L · Status In Progress
Confidence: 0.72 (mixed)

Reasoning

component from path globs (test-coverage, test-framework, ci); effort from diff stats (1591+13 LOC, 15 files); LLM: Adds new $commitTransaction command test coverage and enables replica set in CI, touching both test files and CI workflow configuration.

If a label is wrong, remove it manually and ping @patty-chow so the rules can be tuned. The bot will not re-label items that already have component labels.

[alinaliBQ]

@Rishabh998 Addressed the comments, please have a look

[alinaliBQ]

I have rebased the PR to get the changes for #606. cc @eerxuan

[alinaliBQ]

@eerxuan I have rebased this PR, will continue to rebase other PRs to use @pytest.mark.requires(change_streams=True).

[eerxuan]

[Suggestion] Restructure the shared session helper — it obscures each test's setup/assert flow

Thanks for the thorough API-acceptance coverage here — the field_types / comment / writeConcern / *_error files are clean, one-assertion-per-test, and I don't think they need changes.

My concern is utils/session_test_case.py. execute_session_command bundles the lifecycle, branching, and assertions into one helper, and for only a handful of behavioral tests it carries a lot of complexity:

• Multiple if/else paths (commit_command set vs. not, expected_response vs. readback, abort vs. commit).
• Raw assert statements inside the helper (assert len(docs) > 0, …), which both violate TEST_FORMAT.md ("use helpers from framework.assertions, not plain assert") and split each test's real pass/fail logic across two layers — so you can't read a def test_* and see what it verifies.

Net effect: the actual execution + assertion path is hidden in the helper, and a reader can't tell what any given test is doing.

Suggested format: inline the lifecycle, one assertion per test

The transaction lifecycle is only three lines — put them in the test body so setup → act → assertion is visible top to bottom, no helper needed:

def test_commit_persists_insert(collection):
"""A committed insert is visible outside the transaction afterward."""
with collection.database.client.start_session() as session:
session.start_transaction()
collection.insert_one({"_id": 1, "x": 1}, session=session)
session.commit_transaction()
readback = execute_command(collection, {"find": collection.name, "filter": {}})
assertSuccess(readback, [{"_id": 1, "x": 1}])

def test_insert_rolled_back_on_abort(collection):
"""An insert is gone after the transaction aborts."""
with collection.database.client.start_session() as session:
session.start_transaction()
collection.insert_one({"_id": 1, "x": 1}, session=session)
session.abort_transaction()
readback = execute_command(collection, {"find": collection.name, "filter": {"_id": 1}})
assertSuccess(readback, [])

The with is just to close the session; the test itself calls commit_transaction() / abort_transaction(). execute_session_command, the SessionOp enum, and SessionOperation can all go away.

Use a single test function per spec — no parametrize, and insert is enough

These are behavioral specs, not type matrices, so one plain def test_* each. We don't need to re-run each spec for insert/update/delete — a simple insert proves the behavior; operation-type coverage belongs in the separate file below.

Missing behavioral specs

The PR covers the command API (which inputs are accepted). It doesn't cover full transaction behavior. Listing all specs needed, may exist in this PR or the abortTransaction PR:

Missing behavioral specs (one file per spec, one assertion per test, insert-only)

• Commit persists — committed insert is visible outside the txn after commit
• Isolation (outside) — pending insert is NOT visible to a reader outside the txn before commit
• Visibility (in-session) — pending insert IS visible to a read in the same session before commit
• Rollback on abort — aborted insert leaves no trace
• Commit is retryable — re-sending commitTransaction after a successful commit returns ok:1 (committed state is retained for retryability), not an error
• Cannot commit after abort — commit after abort fails with 251 / NoSuchTransaction
• Failed op blocks the txn — after a duplicate-key error (11000) inside the txn, a later op in the same txn fails with 251 / NoSuchTransaction (must abort to recover)
• Abort recovers the session — after a failed op + abort, the same session can run a new transaction successfully
• Write conflict — a second writer to a doc already modified in an open txn fails with 112 / WriteConflict (first-committer-wins, not blocking)
• Explicit writeConcern on an op inside a txn is rejected — 72 / InvalidOptions (the doc: "Setting write concerns for the individual write operations inside a transaction returns an error")
• Cross-collection atomicity — writing to two different collections in one txn then committing persists both; aborting persists neither (the PR's commit_multi_operation only touches one collection)
• Commit empty transaction — already covered, keep

Separate file for supported/disallowed operations in a transaction
Which operation types are allowed in a transaction is its own concern — put it in a dedicated file rather than folding it into each behavioral test. One test confirming a CRUD op (e.g. find) runs in a txn, and tests that disallowed ops (count, listCollections, listIndexes, explain, createUser) fail.

Disallowed operations inside a transaction

• Write to config / admin / local db → 263 / OperationNotSupportedInTransaction
• Write to a capped collection → 263 / OperationNotSupportedInTransaction
• count command → rejected (use $count aggregation instead)
• listCollections / listIndexes → rejected
• explain → rejected (explicitly not allowed in a txn per the doc)
• createUser → rejected

Special Supported operations inside a transaction (separate file)

• aggregate $count runs in a txn (the supported counterpart to the disallowed count command)

[alinaliBQ]

@eerxuan Addressed the comment. SessionTestCase is removed. Every test now drives the transaction lifecycle inline (no helper, no SessionOp/SessionOperation/SessionTestCase), one plain def test_* per behavior, one assertion each, insert-only.

Changed / added (all under commitTransaction/)

• test_commitTransaction.py — restructured to plain defs: persists_insert, empty, response_ok, is_retryable. Dropped parametrize + SessionTestCase, and dropped the commit_with_writeconcern / commit_with_comment / commit_multi_operation cases (redundant with the writeConcern/comment API files and superseded by the cross-collection atomicity test).
• test_commitTransaction_visibility.py — pending write not visible outsidebefore commit; visible in-session before commit.
• test_commitTransaction_atomicity.py — commit persists writes across two collections.
• test_commitTransaction_supported_operations.py — find, aggregate $count, update, delete run in a txn.
• test_commitTransaction_write_conflict.py — a second writer to a doc modified in an open txn fails with 112 / WriteConflict (first-committer-wins, not blocking).
• test_commitTransaction_op_writeconcern.py — an explicit writeConcern on an operation inside a txn is rejected with 72 / InvalidOptions.
• test_commitTransaction_disallowed_operations.py — count / listCollections / listIndexes / explain / createUser, writes to a capped collection,and writes to the config / local databases are rejected with 263 / OperationNotSupportedInTransaction.
• Deleted the now-unused utils/session_test_case.py.

All behaviors were confirmed against the live replica set before writing assertions.

Notes

• admin db write is actually allowed inside a transaction on this server
  (returns ok, not 263), so the system-database test asserts 263 for config and local only and leaves admin out (asserting real behavior).
• Re-added WRITE_CONFLICT_ERROR = 112 to error_codes.py.

Deferred to the abortTransaction PR (touch abort)

These call abort_transaction() and/or assert post-abort behavior, so they belong with the abort work:

• Rollback on abort
• Abort recovers the session
• Cannot commit after abort (251 / NoSuchTransaction)
• Failed-op-blocks-txn (dup-key 11000 → next op 251, "must abort to recover")
• Cross-collection abort half (aborting persists neither)

[eerxuan]

Please resolve conflict

[alinaliBQ]

Conflict resolved