test: stabilize batched update assertions

[renecannao]

Summary

• consume all contiguous I3/I4 fragments through each expected GTID range
• deliberately cross timer boundaries so the regression is deterministic
• retain checks for correct message ordering, UUID, contiguous coverage, and real aggregation

Root cause

The v2.4.0 release gate assumed five inserts would always be emitted in one 300 ms timer window. On Ubuntu 22 the timer flushed after four inserts, producing a valid I3 followed by I4; the test rejected that valid split and blocked image publication.

Validation

• make -C test/tap/tests test_batched_updates-t
• MYSQL_VERSIONS=84 docker compose -f test/infra/docker-compose.yml run --rm -e MYSQL_SSL_MODE=REQUIRED -e MYSQL_SSL_VERIFY_SERVER_CERT=0 runner test/tap/run.sh

The full dbdeployer MySQL 8.4 TAP suite passed, including the forced split-batch path.

---

Summary by cubic

Stabilizes the batched update test by accepting contiguous I3/I4 sequences that fully cover each expected GTID range and by requiring aggregation only when the burst fits in one timer window. Previously the test demanded exactly one I3 then one I4 per five inserts and failed on valid split or slow-burst emissions.

• Deliberately split each 5-insert group across timer windows (4 inserts, sleep past freq_ms, 5th insert) to make the split deterministic.
• Make assertions timing-aware: compute whether aggregation is required; pass if coverage is contiguous and complete, and when required, at least one interval aggregates multiple trxids.
• Add helpers: read_batched_range, join_lines, cross_timer_boundary, and aggregation_is_required. Validate UUID, ordering, contiguity, and coverage across one or more lines; first group starts with I3, second group emits I4 only.

^{Written for commit 72d9d46. Summary will update on new commits.}

Summary by CodeRabbit

• Tests
  • Expanded coverage for batched updates spanning timer boundaries.
  • Added validation that row ranges remain contiguous and complete across multiple update batches.
  • Replaced limited batch assertions with broader checks covering successive groups of inserted rows.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5c42faa5-ceb3-4c48-b50c-25018c090bbb

📥 Commits

Reviewing files that changed from the base of the PR and between 0155773 and 38fcba9.

📒 Files selected for processing (1)

• test/tap/tests/test_batched_updates-t.cpp

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details

⏰ Context from checks skipped due to timeout. (7)

• GitHub Check: package (ubuntu22)
• GitHub Check: package (centos10)
• GitHub Check: package (ubuntu24)
• GitHub Check: package (debian13)
• GitHub Check: package (centos9)
• GitHub Check: test
• GitHub Check: Gitar

🧰 Additional context used

🪛 ast-grep (0.45.1)

test/tap/tests/test_batched_updates-t.cpp

[error] 151-152: SQL query built with std::string/QString concatenation is passed to a database execution sink (sqlite3_exec, mysql_query, or QSqlQuery::exec). Concatenating untrusted values into SQL enables SQL injection. Use parameterized queries / prepared statements instead (e.g. sqlite3_prepare_v2 with sqlite3_bind_*, mysql_stmt_prepare/mysql_stmt_bind_param, or QSqlQuery::prepare with addBindValue/bindValue).
Context: db.exec("INSERT INTO binlog_reader_test.batching_t (v) VALUES (" +
std::to_string(i) + ")")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(sql-injection-string-concat-cpp)

---

[error] 170-171: SQL query built with std::string/QString concatenation is passed to a database execution sink (sqlite3_exec, mysql_query, or QSqlQuery::exec). Concatenating untrusted values into SQL enables SQL injection. Use parameterized queries / prepared statements instead (e.g. sqlite3_prepare_v2 with sqlite3_bind_*, mysql_stmt_prepare/mysql_stmt_bind_param, or QSqlQuery::prepare with addBindValue/bindValue).
Context: db.exec("INSERT INTO binlog_reader_test.batching_t (v) VALUES (" +
std::to_string(i) + ")")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(sql-injection-string-concat-cpp)

🔇 Additional comments (3)

test/tap/tests/test_batched_updates-t.cpp (3)

4-22: LGTM!

---

41-104: LGTM!

---

148-186: LGTM!

---

📝 Walkthrough

Walkthrough

The batched update test now covers inserts split across timer boundaries. It adds helpers for reading and validating contiguous I3/I4 ranges, then verifies aggregation for two five-row insertion scenarios.

Changes

Batched update validation

Layer / File(s)	Summary
Range helpers and timer control test/tap/tests/test_batched_updates-t.cpp	The test adds timer-related includes, batched range validation, raw output capture, reader error checks, and timer-boundary delays.
Timer-split insertion scenarios test/tap/tests/test_batched_updates-t.cpp	The test replaces single-message assertions with two scenarios that insert rows across timer intervals and validate complete contiguous I3/I4 coverage.

Estimated code review effort: 2 (Simple) | ~15 minutes

Merge Risk: ⚪ Minimal · up to 38fcb

This localized test-only change stabilizes batched update assertions without changing product runtime behavior. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: wazir-ahmed

Poem

I hop through timers, neat and bright,
Five rows gather past the night.
I3 and I4 trace each way,
Contiguous ranges end the day.
— A test rabbit 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: stabilizing batched update assertions in the test.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/release-batching-test

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gitar-bot]

Code Review ✅ Approved 1 resolved / 1 findings

Refactors batched update test assertions to accept contiguous I3/I4 sequences across timer boundaries, addressing the residual timer-batching assumption finding. No issues found.

✅ 1 resolved

✅ Edge Case: aggregated assertion retains a residual timer-batching assumption

📄 test/tap/tests/test_batched_updates-t.cpp:82 📄 test/tap/tests/test_batched_updates-t.cpp:164 📄 test/tap/tests/test_batched_updates-t.cpp:183
read_batched_range only sets aggregated=true when some emitted interval spans more than one trxid, and both ok() checks require b1.aggregated/b2.aggregated. If the independent 300ms timer happens to flush every insert individually (each interval start==end), the coverage is still contiguous and complete but aggregated stays false and the test fails — the same class of timer-phase flakiness this PR sets out to eliminate. In practice the four back-to-back inserts before the boundary make this very unlikely, so this is minor; consider relaxing to assert only complete (contiguity + coverage) if any future flakiness surfaces.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

---

Important

Your trial ends in 4 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

_{Was this helpful? React with 👍 / 👎 | Gitar}