Fix review issues in FFTO E2E tests (#5516)

Two critical issues identified during code review:

1. test_ffto_mysql_transactions-t.cpp: TAP plan count mismatch.
   kPlannedTests was declared as 28 but the actual assertion count
   is 34 (1 connect + 9 scenario1 + 12 scenario2 + 3 scenario3 +
   9 scenario4). This would cause TAP to report "planned 28 but
   ran 34" and fail unconditionally. Fixed by correcting the count
   to 34 and cleaning up the misleading breakdown comment.

2. test_ffto_mysql_concurrent-t.cpp: undefined behavior on
   pthread_create failure. If pthread_create failed for thread i,
   threads[i] was never initialized, but pthread_join(threads[i])
   was still called unconditionally. Fixed by tracking which threads
   were successfully created via a bool array and only joining those.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
v3.0-5516
Rene Cannao 2 months ago
parent 7949016f62
commit b16d1f44ee

@ -274,6 +274,7 @@ int main(int argc, char** argv) {
thread_args args[NUM_THREADS];
pthread_t threads[NUM_THREADS];
bool thread_created[NUM_THREADS] = {};
for (int i = 0; i < NUM_THREADS; i++) {
args[i].thread_id = i;
@ -286,11 +287,15 @@ int main(int argc, char** argv) {
diag("Failed to create thread %d", i);
std::lock_guard<std::mutex> lock(tap_mutex);
ok(0, "Thread %d creation failed", i);
} else {
thread_created[i] = true;
}
}
for (int i = 0; i < NUM_THREADS; i++) {
pthread_join(threads[i], NULL);
if (thread_created[i]) {
pthread_join(threads[i], NULL);
}
}
/* ── Aggregate stats verification ───────────────────────────────── */

@ -35,31 +35,15 @@
/**
* @brief Total number of planned TAP assertions.
*
* Breakdown:
* - Setup: 1 (connect)
* - Scenario 1 (BEGIN/COMMIT): 4×3 = 12 (BEGIN, INSERT, INSERT digests, COMMIT)
* INSERT shares one digest with count=2
* actually 3 digests × 3 assertions = 9
* - Scenario 2 (ROLLBACK): 4×3 = 12 4 digests × 3 = 12
* but BEGIN reuses scenario 1 digest
* 3 new digests × 3 + reuse = still verify 4
*
* Simplified: we verify per-scenario after clearing stats each time.
* - Scenario 1: 3 verify_digest calls = 9 assertions
* - Scenario 2: 4 verify_digest calls = 12 assertions
* - Scenario 3: 1 verify_digest call = 3 assertions
* - Scenario 4: 3 verify_digest calls = 9 assertions
* Breakdown (stats cleared between scenarios):
* - Setup: 1 (connect ok)
* - Scenario 1: 3 verify_digest calls (BEGIN, INSERT, COMMIT) = 9
* - Scenario 2: 4 verify_digest calls (BEGIN, INSERT, SELECT, ROLLBACK) = 12
* - Scenario 3: 1 verify_digest call (prepared INSERT ×20) = 3
* - Scenario 4: 3 verify_digest calls (SAVEPOINT, INSERT, RELEASE) = 9
* Total = 1 + 9 + 12 + 3 + 9 = 34
*
* That's too many let's trim to essential checks:
* - Scenario 1: verify BEGIN(1), INSERT(2 affected), COMMIT(1) = 9
* - Scenario 2: verify ROLLBACK(1), SELECT(1 row_sent) = 6
* - Scenario 3: verify prepared INSERT (20 executions, 20 affected) = 3
* - Scenario 4: verify SAVEPOINT(1), INSERT(1), RELEASE(1) = 9
* + 1 connect
* Total = 28
*/
static constexpr int kPlannedTests = 28;
static constexpr int kPlannedTests = 34;
/** @copydoc FAIL_AND_SKIP_REMAINING */
#define FAIL_AND_SKIP_REMAINING(cleanup_label, fmt, ...) \

Loading…
Cancel
Save