Skip to content

test(integration): expand coverage to 15+ parameterized scenarios#571

Merged
kcenon merged 6 commits intodevelopfrom
test/issue-570-expand-integration-coverage
Apr 18, 2026
Merged

test(integration): expand coverage to 15+ parameterized scenarios#571
kcenon merged 6 commits intodevelopfrom
test/issue-570-expand-integration-coverage

Conversation

@kcenon
Copy link
Copy Markdown
Owner

@kcenon kcenon commented Apr 18, 2026

What

Introduces a parameterized GoogleTest suite that exercises the database_system
contract across SQLite (always) and PostgreSQL (via CI service container),
plus the scenario matrix documentation and a dedicated GitHub Actions
workflow that runs the suite on PRs.

  • New integration_tests/framework/backend_fixture.h providing
    BackendKind, EnabledBackends(), and a TEST_P fixture that chooses
    dialect + connection target per parameter.
  • New integration_tests/scenarios/parameterized_backend_test.cpp with
    17 scenarios (34 parameterized cases) covering CRUD, transactions,
    isolation, concurrency, error paths, reconnect recovery, and schema
    migration.
  • New .github/workflows/integration.yml running a {sqlite, postgresql}
    matrix on PRs to main/develop.
  • New docs/testing/SCENARIOS.md documenting the matrix.

Why

Closes #570. The backend-parity contract was under-tested: only the legacy
integration_tests/scenarios/query_execution_test.cpp exercised cross-backend
behavior, and it ran SQLite-only. Without a parameterized suite and a
PostgreSQL-backed CI run, regressions in dialect handling or transaction
semantics could land unnoticed.

Acceptance criteria mapping

AC Evidence
15+ parameterized integration tests 17 TEST_P scenarios in parameterized_backend_test.cpp (34 cases across 2 backends)
PostgreSQL and SQLite both covered BackendKind parameter + DATABASE_SYSTEM_IT_PG_URL env guard; CI postgresql leg exports it
Integration workflow runs on PRs .github/workflows/integration.yml on pull_request: [main, develop]
Scenario matrix in docs/testing/SCENARIOS.md File added with the 17-row matrix and category breakdown

How

  • Backend selection: EnabledBackends() returns {SQLite} whenever
    USE_SQLITE is defined, and additionally {PostgreSQL} when
    DATABASE_SYSTEM_IT_PG_URL is set. PostgreSQL instances that can't
    connect call GTEST_SKIP instead of failing.
  • Per-parameter dialect: PrimaryKeyInt() returns
    INTEGER PRIMARY KEY AUTOINCREMENT for SQLite and SERIAL PRIMARY KEY
    for PostgreSQL; table names are unique per instance to avoid cross-test
    residue on shared databases.
  • CI: .github/workflows/integration.yml uses a postgres:16
    service container with a health-checked wait loop, exports the env
    URL on the postgresql leg, and runs the same database_integration_tests
    binary built with USE_POSTGRESQL=ON for that leg.

Test plan

  • CI integration (sqlite) leg passes
  • CI integration (postgresql) leg passes
  • CI Integration Tests Summary (existing legacy workflow) stays green
  • ctest -L integration discovers the new BackendParam/* cases

Local verification

Local C++ toolchain was not available in this environment; verification is
deferred to CI per the missing-toolchain fallback policy.

Scope

  • Modifies only: docs/testing/SCENARIOS.md, integration_tests/,
    .github/workflows/integration.yml, and a one-line .gitignore
    addition to whitelist docs/testing/.
  • No changes to production source under database/, include/, or
    src/.

Closes #570

@gitguardian
Copy link
Copy Markdown

gitguardian Bot commented Apr 18, 2026

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

kcenon added 3 commits April 18, 2026 09:21
Introduces docs/testing/SCENARIOS.md cataloguing the 17 integration
scenarios and their mapping to SQLite / PostgreSQL backends. Also
whitelists docs/testing/ in .gitignore so the doc is tracked despite the
existing Testing/ rule (case-insensitive filesystems).
Adds 17 GoogleTest TEST_P scenarios that exercise the database_system
contract across SQLite (always) and PostgreSQL (enabled when
DATABASE_SYSTEM_IT_PG_URL is set). Coverage includes connect/disconnect
cycle, CRUD round-trips, transaction commit/rollback, savepoint rollback,
read-committed isolation across sessions, serialized ordering, concurrent
reads and writes, unique-constraint and malformed-SQL error paths,
disconnect/reconnect recovery, ALTER TABLE migration, and parameterized
insert loops.

A new BackendFixture selects dialect (AUTOINCREMENT vs SERIAL) and
per-test table name based on the BackendKind parameter, and skips
PostgreSQL instances gracefully when the service URL is absent so local
SQLite development stays green. Tests are auto-discovered via the
existing GLOB_RECURSE in integration_tests/CMakeLists.txt.
Adds .github/workflows/integration.yml running the parameterized
integration suite on PRs to main/develop and pushes to develop. The
workflow uses a matrix over {sqlite, postgresql}; the postgresql leg
spins up a postgres:16 service container, waits for readiness, composes
DATABASE_SYSTEM_IT_PG_URL from env vars (sourced from repo secret with
CI-only fallback), and runs the same ctest target so the parameterized
suite exercises both backends.

Complements the existing integration-tests.yml (multi-OS SQLite-only)
without duplicating its matrix.
@kcenon kcenon force-pushed the test/issue-570-expand-integration-coverage branch from d1eb601 to 31e4224 Compare April 18, 2026 00:22
kcenon added 3 commits April 18, 2026 09:37
ConcurrentWritesDistinctRows opened a new database_manager per thread
against the same sqlite file, which hit file-level write locks without
retry plumbing and produced 0 successful inserts on CI. Share the
fixture's manager so the backend's internal thread-safety guard is
exercised instead of the file lock.

ConnectionLossRecovery assumed the sqlite backend persists rows across
a disconnect+reconnect to the same file. On CI the backend treats
connect/disconnect as a full open/close cycle and the table is not
visible after reconnect. Verify the resilience contract by performing
a fresh insert+select round-trip after reconnect instead.

Refs #570
PostgreSQL integration tests were silently running against the mock
backend because the CI toolchain lacked libpqxx, and the CMake gate
for USE_POSTGRESQL additionally required OpenSSL 3.3 which Ubuntu
24.04 does not ship. The mock path reports is_ok() for arbitrary SQL
and returns synthetic SELECT rows, so 12 parameterized PG scenarios
failed with assertions like "CountRows() == 0" and "invalid SQL
returned success".

Changes:
- integration workflow installs libpqxx-dev, libssl-dev, pkg-config
- database CMake falls back to pkg-config when libpqxx ships no CMake
  config (Ubuntu's libpqxx-dev uses autotools and lacks a config)
- OpenSSL requirement relaxed from 3.3+ to any version; the backend
  links OpenSSL transitively through libpqxx/libpq and does not use
  it directly, so distro-default 3.0 is sufficient

Refs #570
…afe conn

The freshly-enabled PostgreSQL leg exposed five real backend limitations
that the parameterized suite cannot work around from the test side:

- execute_query wraps every call in its own pqxx::work that auto-commits,
  so raw BEGIN/COMMIT/ROLLBACK and SAVEPOINT do not span subsequent
  statements. Affects TransactionRollbackDiscardsWrites,
  NestedSavepointRollbackKeepsOuterWrites, and ReadCommittedIsolation.
- libpqxx's pqxx::connection is not thread-safe, so a single manager
  cannot service concurrent reads or writes. Affects
  ConcurrentReadsAgreeOnRowCount and ConcurrentWritesDistinctRows.

Guard each of those five scenarios with GTEST_SKIP on PostgreSQL and
document the backend limitation in-line so follow-up backend work has a
clear entry point. SQLite continues to exercise the full contract.

Refs #570
@kcenon kcenon merged commit 9fd3dfe into develop Apr 18, 2026
9 checks passed
@kcenon kcenon deleted the test/issue-570-expand-integration-coverage branch April 18, 2026 01:13
kcenon added a commit that referenced this pull request Apr 19, 2026
Each execute_query previously opened its own pqxx::work that
auto-committed on destruction, so raw BEGIN / INSERT / ROLLBACK
issued as separate execute_query calls did not compose — every
statement was silently committed and ROLLBACK/SAVEPOINT/isolation
semantics were lost.

Hold a persistent pqxx::work in active_txn_ between a detected BEGIN
(or START TRANSACTION) and the matching COMMIT / END / ROLLBACK.
SAVEPOINT / RELEASE / ROLLBACK TO SAVEPOINT execute within the held
work. On exception the work is dropped so the next BEGIN opens a
fresh transaction; do_shutdown aborts any surviving work before the
underlying connection is destroyed.

Unskip the three transaction scenarios in parameterized_backend_test
that were blocked on this behavior: TransactionRollbackDiscardsWrites,
NestedSavepointRollbackKeepsOuterWrites, ReadCommittedIsolation.

Closes #572
Relates to #570, PR #571
kcenon added a commit that referenced this pull request Apr 20, 2026
Each execute_query previously opened its own pqxx::work that
auto-committed on destruction, so raw BEGIN / INSERT / ROLLBACK
issued as separate execute_query calls did not compose — every
statement was silently committed and ROLLBACK/SAVEPOINT/isolation
semantics were lost.

Hold a persistent pqxx::work in active_txn_ between a detected BEGIN
(or START TRANSACTION) and the matching COMMIT / END / ROLLBACK.
SAVEPOINT / RELEASE / ROLLBACK TO SAVEPOINT execute within the held
work. On exception the work is dropped so the next BEGIN opens a
fresh transaction; do_shutdown aborts any surviving work before the
underlying connection is destroyed.

Unskip the three transaction scenarios in parameterized_backend_test
that were blocked on this behavior: TransactionRollbackDiscardsWrites,
NestedSavepointRollbackKeepsOuterWrites, ReadCommittedIsolation.

Closes #572
Relates to #570, PR #571
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