Skip to content

Support parallel test runs in external-schema mode#3512

Merged
jonathangreen merged 3 commits into
mainfrom
chore/parallel-external-schema-tests
Jun 29, 2026
Merged

Support parallel test runs in external-schema mode#3512
jonathangreen merged 3 commits into
mainfrom
chore/parallel-external-schema-tests

Conversation

@jonathangreen

@jonathangreen jonathangreen commented Jun 29, 2026

Copy link
Copy Markdown
Member

Description

Makes the test suite's external-schema mode parallel-capable. When database creation is enabled (the default), each xdist worker now provisions its own database by cloning the configured database as a Postgres template (CREATE DATABASE ... TEMPLATE), instead of all workers sharing the one configured database. The previous release's tests still run against the schema the current code produced, but each worker gets an isolated database, so the suite can run under xdist.

Admin (CREATE/DROP DATABASE) operations now connect to the postgres maintenance database rather than the configured one, since you cannot clone a template you are connected to; these are cluster-level commands that work from any database, and this path is exercised by the normal suite on every run.

The single-shared-database serial mode (external schema with database creation disabled) is preserved for local debugging.

Motivation and Context

The backwards-compatibility CI check runs a previous release's -m db test suite against a schema built by the current code, using external-schema mode. Until now that mode used the one configured database as-is and forbade xdist, so the suite ran serially (-n0) and took ~13 minutes. Its not really feasible for this check to take so long, so I'm adding this option to speed it up.

This will have to be merged and go into a release, before I can test it in the workflow in #3506

How Has This Been Tested?

Because the backwards-compatibility job won't exercise this for real until the next release, it was proven locally:

  • Parallel external clone mode against a real Postgres: a db-test slice ran green under -n auto with PALACE_TEST_DATABASE_EXTERNAL_SCHEMA=true / PALACE_TEST_DATABASE_CREATE_DATABASE=true; per-worker session_gw* clones were created from the template and dropped at teardown.
  • Negative control: dropping a table from the template made the same parallel run fail with UndefinedTable, confirming the check still catches backwards-incompatible schemas under parallelism.
  • Normal-mode regression: the standard parallel suite (which now routes admin DDL through the postgres maintenance database on every run) passed via tox -e py312-docker.
  • Full end-to-end against a locally built image using the real backwards-compatibility compose overlay (-n auto, clone-per-worker): 3,269 db tests passed in 3:47 on a 4-worker runner, versus ~13 minutes serial.
  • mypy clean.

Checklist

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

The backwards-compatibility CI check runs a previous release's `-m db` test
suite against a schema built by the current code, using external-schema mode
(`PALACE_TEST_DATABASE_EXTERNAL_SCHEMA=true`). Until now that mode used the one
configured database as-is and forbade xdist, so the suite had to run serially
(`-n0`) and took ~13 minutes.

Make external-schema mode parallel-capable: when database creation is enabled
(the default), each worker now provisions its own database by cloning the
configured database as a Postgres template (`CREATE DATABASE ... TEMPLATE`). The
previous release's tests still run against the schema the current code produced,
but each worker gets an isolated database, so the suite can run under xdist.

Admin (CREATE/DROP DATABASE) operations now connect to the `postgres`
maintenance database rather than the configured one, since you cannot clone a
template you are connected to; these are cluster-level commands that work from
any database, and this path is exercised by the normal suite on every run.

The single-shared-database serial mode (external schema with database creation
disabled) is preserved for local debugging.
@claude

claude Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @jonathangreen's task in 2m 8s —— View job


Claude Code Review

  • Read the PR diff against origin/main
  • Review tests/fixtures/database.py changes
  • Review README.md changes
  • Assess the open Greptile review comment
  • Post review findings

Summary

This is a clean, well-scoped change confined to the test fixture layer. The template-clone approach (CREATE DATABASE … TEMPLATE) is the standard PostgreSQL mechanism for per-worker isolation, and routing admin DDL to the postgres maintenance database is the correct prerequisite (you can't clone a template you're connected to). The three modes are all coherent: normal parallel (clone from models), external-schema parallel (clone the externally-built database), and external-schema serial (create_database=false, used directly). The integration is also correct downstream — DatabaseFixture.fixture (tests/fixtures/database.py:439-443) skips the drop/recreate in external-schema mode, so each worker correctly runs against its cloned copy of the externally-built schema. Comments and README.md accurately describe the new behavior. I found no actionable issues.

Note on the prior review

The Greptile P1 ("Missing None guard for template database name") has already been addressed by commit 24f5334__init__ now raises a clear BasePalaceException at setup time when external_schema and create_database and self._config_url.database is None (tests/fixtures/database.py:226-234), so the CREATE DATABASE … TEMPLATE None path the comment described is no longer reachable.
chore/parallel-external-schema-tests

@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR enables parallel test execution in external-schema mode by cloning the configured database as a PostgreSQL template per xdist worker (CREATE DATABASE ... TEMPLATE), rather than sharing a single database serially. Admin DDL (CREATE/DROP DATABASE) is now routed through the postgres maintenance database on every run so that nothing remains connected to the template during cloning.

  • Removes the old mutual-exclusion guard between external_schema=True and create_database=True, replacing it with a targeted null-check on the template database name.
  • Switches _db_connection to always connect to the postgres maintenance database (instead of the configured URL), adding a new connection prerequisite for the test user.
  • Documentation in README.md is updated to reflect the new default-parallel behaviour and the retained serial path.

Confidence Score: 5/5

Safe to merge. The logic is correct and well-reasoned; the two observations are minor hardening points for edge-case configurations.

The core changes — removing the external_schema/create_database mutual-exclusion guard, routing admin DDL through the postgres maintenance database, and template-cloning per worker — are all sound. The null guard for the template name was added correctly. The PR was exercised locally against a real Postgres cluster in both parallel and serial modes with positive results. The two observations affect only non-standard database naming or managed-cloud setups, neither of which applies to the project's Docker-based CI.

No files require special attention.

Important Files Changed

Filename Overview
tests/fixtures/database.py Core logic change: admin connections now always target the postgres maintenance database; external-schema clone path added with a null guard on the template name. Logic is correct for typical deployments; the template name is interpolated as an unquoted SQL identifier.
README.md Documentation updated to accurately reflect the new default-parallel external-schema behaviour and the retained serial (create_database=false) path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[DatabaseCreationFixture.__init__] --> B{create_database?}
    B -- false --> C{parallel?}
    C -- yes --> D[raise: serial-only]
    C -- no --> E[use configured URL as-is]
    B -- true --> F{external_schema?}
    F -- yes --> G{config URL has database?}
    G -- no --> H[raise: template name required]
    G -- yes --> I[_create_db: CREATE DATABASE worker_db TEMPLATE config_db]
    F -- no --> J[_create_db: CREATE DATABASE worker_db]
    I --> K[GRANT ALL PRIVILEGES TO user]
    J --> K
    K --> L[Tests run against worker_db]
    L --> M[_drop_db: DROP DATABASE worker_db]

    subgraph _db_connection
        N[connect to postgres maintenance DB with AUTOCOMMIT]
    end
    I -.-> N
    J -.-> N
    M -.-> N
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[DatabaseCreationFixture.__init__] --> B{create_database?}
    B -- false --> C{parallel?}
    C -- yes --> D[raise: serial-only]
    C -- no --> E[use configured URL as-is]
    B -- true --> F{external_schema?}
    F -- yes --> G{config URL has database?}
    G -- no --> H[raise: template name required]
    G -- yes --> I[_create_db: CREATE DATABASE worker_db TEMPLATE config_db]
    F -- no --> J[_create_db: CREATE DATABASE worker_db]
    I --> K[GRANT ALL PRIVILEGES TO user]
    J --> K
    K --> L[Tests run against worker_db]
    L --> M[_drop_db: DROP DATABASE worker_db]

    subgraph _db_connection
        N[connect to postgres maintenance DB with AUTOCOMMIT]
    end
    I -.-> N
    J -.-> N
    M -.-> N
Loading

Reviews (3): Last reviewed commit: "Clarify the create_database comment" | Re-trigger Greptile

Comment thread tests/fixtures/database.py
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.44%. Comparing base (e9110ae) to head (dab6243).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3512   +/-   ##
=======================================
  Coverage   93.44%   93.44%           
=======================================
  Files         511      511           
  Lines       46468    46468           
  Branches     6340     6340           
=======================================
  Hits        43423    43423           
  Misses       1969     1969           
  Partials     1076     1076           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

In external-schema mode the configured database is used as the clone template.
If the configured URL omits a database component, the previous code would emit
`CREATE DATABASE foo TEMPLATE None` and fail with an obscure PostgreSQL error.
Validate at fixture setup that the URL names a database, raising a descriptive
BasePalaceException instead.
Fix a typo and use "worker" rather than "thread" (xdist runs separate worker
processes), and tidy the wording describing how each worker's database is
populated in normal vs. external-schema mode.
@jonathangreen jonathangreen requested a review from a team June 29, 2026 14:56

@dbernstein dbernstein left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦞 looks good.

@jonathangreen jonathangreen merged commit c1332fe into main Jun 29, 2026
23 checks passed
@jonathangreen jonathangreen deleted the chore/parallel-external-schema-tests branch June 29, 2026 23:51
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.

2 participants