Skip to content

Create the shared test fixtures once per pytest run - #878

Closed
laughingman7743 wants to merge 6 commits into
masterfrom
test/848-shared-fixture-schema
Closed

laughingman7743 wants to merge 6 commits into
masterfrom
test/848-shared-fixture-schema

Conversation

@laughingman7743

@laughingman7743 laughingman7743 commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

WHAT

The PyAthena suite now creates its read-only fixtures once per pytest run, in one schema that every pytest-xdist worker shares.

  • tests/__init__.py
    • Adds ENV.fixture_schema, a second name of the pyathena_test_<10 [a-z0-9]> shape that scripts/sweep_databases.py sweeps.
    • ENV.schema stays per process, and so does the S3 Tables namespace.
    • ENV.s3_filesystem_test_file_key moves under the fixture schema's prefix. The filesystem tests only read that key.
  • tests/pyathena/conftest.py
    • The pytest-xdist controller creates the fixture schema in pytest_sessionstart: data upload, database, 7 tables and 2 views. It drops the schema in pytest_sessionfinish.
    • The controller passes the schema name to the workers through workerinput. Its pytest_configure_node hook is registered only when the xdist plugin is present.
    • A worker sets ENV.fixture_schema in pytest_configure, and creates and drops only its own empty ENV.schema and S3 Tables namespace.
    • A run without workers creates both schemas.
    • Each removal is recorded before its create step. pytest_sessionfinish runs them; a pytest-xdist worker reports that it finished only after that hook. pytest_sessionstart is a wrapper that runs them at once when it or a later session-start hook fails, before the error reaches pytest-xdist. A config cleanup covers a failure outside the wrapper. The list is cleared before it runs, so every removal is attempted once.
    • Guard: before dropping the fixture schema, its owner lists the schema's tables with Glue GetTables. If they differ from TABLES/VIEWS, or the schema no longer exists, it reports the unexpected and missing names and fails the run.
    • The cursor, engine, and aio fixtures now default to ENV.fixture_schema, so the unqualified reads of one_row, many_rows, and the other fixtures are unchanged.
  • Tests
    • Fixture reads that were qualified or asserted with ENV.schema now use ENV.fixture_schema. This covers the reflection and location assertions, the view definition, the schema-wide Glue/Athena listing comparisons, and the Spark reads of one_row and the CSV.
    • Writes stay in ENV.schema.
    • The only unqualified accesses to test-created tables were the read-backs in pandas/test_util.py's to_sql tests; they are now qualified.
  • Docs: docs/testing.md describes the two schemas, the controller's role, and the guard. One docstring in scripts/sweep_databases.py is updated.

WHY

Part of #848 (step 3, second part), for #834. Based on master after #873 and #874.

Setup per pytest -n 8 job of the PyAthena suite:

Athena statements S3 PutObject / DeleteObject
Before 88 = 8 workers × (CREATE DATABASE + 7 tables + 2 views + DROP DATABASE) 56 / 56
After 27 = 11 on the controller + 8 × (CREATE DATABASE + DROP DATABASE) 7 / 7, plus 1 Glue GetTables

The fixture schema is read-only during the run. The tests that compare two listings of it therefore stay stable even though all workers read it.

TEST

Tested commit: 01b24f4 for the suite runs below, and d48df7f for the lifecycle repairs from the independent review (see the last bullets).

  • just lint passed: ruff, mypy, cfn-lint, and the license headers. markdownlint-cli2 docs/testing.md reported 0 errors.
  • pytest --collect-only -q tests/pyathena collects 2033 tests, both before and after this change.
  • Against AWS, from this worktree:
    • pytest -n 4 on a -k selection covering every modified test and the tests that read the fixtures: 781 passed, 3 skipped.
    • -p no:xdist and -n 1, each on five tests: 5 passed each. In the -n 1 run, the worker used the controller's fixture schema.
    • Spark with -p no:xdist: 4 passed, 2 skipped.
      • The skipped tests are the sync and aio test_spark_sql. Their pytest.mark.dependency(depends="test_spark_dataframe") passes a string instead of a list, so they are always skipped, with or without xdist.
      • This is an existing problem that this PR does not change.
    • Guard: a temporary test ran an unqualified CTAS into the fixture schema. The run reported unexpected: ['guard_probe'] and exited 1, both with -p no:xdist and with -n 1. The test was not committed.
    • Setup failure: with an invalid AWS_ATHENA_S3_TABLES_CATALOG, the original ValueError surfaced and the fixture schema created before it was dropped.
    • Cleanup: Glue databases and S3 Tables namespaces with the pyathena_test_ prefix were compared before and after all runs. There were 1576 databases and 489 namespaces both times, and the fixture schemas' S3 prefixes were empty.
  • Lifecycle repairs (independent review), against AWS with a helper plugin that recorded every schema name, also checking all pyathena_test_ databases and S3 Tables namespaces created in the last 10 minutes. Nothing was left in any scenario:
    • normal -n 2 (3 and 11 tests): passed;
    • -p no:xdist with a trylast session-start hook that raises: the run aborted;
    • -n 1 with the worker's database creation raising after its namespace was created: the run aborted;
    • -n 1 with a trylast session-start hook raising on the worker: the run aborted;
    • a temporary test that dropped the fixture schema: the guard reported every table missing and the run exited 1 (the test was not committed).
  • After rebasing onto Define the shared test tables in Python and generate their data #874 without Derive the one_row_complex expectations of the Python-object cursors #875–Add an ARRAY<string> column to one_row_complex through its definition only #877 (closed), which restores the hand-written expectations, pytest -n 4 tests/pyathena -k "complex or as_pandas or reflect or table_names or view_names or glue or throttl or table_metadata or to_sql or executemany or show_partition": 238 passed, 1 skipped. The only conflict was test_reflect_select, which keeps its hand-written assertions and reads ENV.fixture_schema.
  • After rebasing onto master (9d34a4b, adding Let explicit cursor() arguments override cursor_kwargs #872 and Replace name-mangled helpers with single-underscore methods #881, which do not touch the fixtures), pytest -n 4 on the connection tests and a test_cursor.py subset: 26 passed.
  • Not run locally: the full suites with -n 8. When the PR is Ready, AWS CI runs the PyAthena suite with Spark and both SQLAlchemy compliance suites, because the PR changes paths under tests/pyathena/sqlalchemy/ and tests/pyathena/spark/.

🤖 Generated with Claude Code

return hasattr(config, "workerinput") or not getattr(config.option, "numprocesses", None)


def _owns_fixture_schema(config):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review round one (implementation behavior): CLEAN

Base bcbf874 (#877 head, the stacked base), head 01b24f4. Full diff reviewed (25 files). A delegated subagent implemented the change; I reviewed the diff myself before publishing.

  • Roles:
    • The controller: _owns_fixture_schema true, _is_test_process false, so it creates only the fixture schema.
    • A worker with the name from the controller: owns false, test process true, so it creates only its own schema.
    • A run without workers, or a worker without the name: owns and runs tests, so it creates both.
    • xdist 3.6.1 order, verified in .venv: the conftest pytest_sessionstart runs before the trylast DSession.pytest_sessionstart starts the nodes. workerinput is set before pytest_configure on workers. The controller finishes its session after every worker's workerfinished.
  • Hook registration: pytest_configure_node lives on _XDistHooks and is registered only with hasplugin("xdist") in a non-worker, so -p no:xdist does not fail hook validation.
  • Failure paths:
    • Setup registers each cleanup before its create step, runs the registered cleanups in reverse under suppress(Exception), and re-raises the original error.
    • Teardown uses _run_all, an ExitStack that runs every step and raises the last failure with the earlier ones chained.
    • The guard lists the fixture schema before the drop. A listing error is only reported.
  • State:
    • _data_objects is cached and used only by the owner, after pytest_configure.
    • s3_filesystem_test_file_key is a property, so workers resolve it against the fixture schema they received.
    • Parametrize decorators do not use either schema, so collection is unaffected.
  • Tests:
    • Every write was already qualified with ENV.schema, and the only unqualified reads of test-created tables (pandas/test_util.py to_sql) are now qualified.
    • The listing tests of the fixture schema are stable because the guard enforces it is read-only.
    • Tests that create an object and then list it use ENV.schema for both.

No findings.

_run_all([functools.partial(_drop_database, ENV.schema), _delete_s3tables_namespace])


def _check_fixture_schema(session):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review round two (claims, callers, AWS operations): FINDINGS (process only; description replaced)

Base bcbf874, head 01b24f4.

Claims checked

  • 27 vs 88 statements, 7 vs 56 puts: 5 data files (one_row, many_rows, one_row_complex, integer_na_values, boolean_na_values) + the Spark CSV + test.dat = 7 objects once. The controller runs 1 CREATE + 7 tables + 2 views + 1 DROP; each of 8 workers runs 1 CREATE + 1 DROP.
  • Docs, "the controller loads these session hooks only when a path given to pytest is tests/pyathena/ or inside it": pytest loads initial conftests for each argument's directory, its parents, and test* subdirectories of the argument. pyathena does not match test*, so pytest tests/ or no argument leaves the controller without this conftest, and the worker fallback applies.
  • Workflow comments: test.yaml:139 ("The three suites create their own schemas and tables") and test-suite.yaml:52 ("each test session creates and deletes its own namespace") are still true.
  • CI scope: tests/pyathena/sqlalchemy/ and tests/pyathena/spark/ changed, so on Ready CI runs the PyAthena suite with Spark and both compliance suites.
  • IAM: the guard needs glue:GetTables on the fixture database, which the existing Glue tests already use.

Operational note on this PR itself: a failed write of the body file let gh pr create publish an unrelated draft from ~/tmp (another repository's PR text) as this PR's body for about a minute. It was replaced with the correct description. The previous text remains in GitHub's edit history.

_create_tables(cursor)
if _owns_fixture_schema(session.config):
cleanups.append(_drop_fixture_schema)
_create_fixture_schema()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent review (relayed Codex result): FINDINGS (2 × P2, 1 × P3), finding 1 of 3

  • Reviewer: Codex CLI 0.157.1 (codex exec -s read-only), model gpt-6-astra, session 01a0e807-6c97-7970-8160-7b4412af877e.
  • Revisions: base bcbf874, head 01b24f4.
  • Setup: the review ran on a detached snapshot without .env and without PR framing. Afterwards the snapshot was unchanged at the head.
  • Limits: static review only. .venv was absent, so the reviewer reasoned from pytest 9 / pytest-xdist 3.6.1 behavior.
  • Coverage, as reported:
    • the controller, worker, and serial lifecycles, including -p no:xdist, -n 0/1/8, collection, interruptions, setup failures, and reruns;
    • schema reads and writes, and the metadata assertions;
    • cleanup of databases, namespaces, and S3 objects;
    • sweep patterns, workflows, the justfile, and the docs.

P2 — Shared resources leak when worker startup fails. This hook creates the shared database and uploads its objects before xdist's trylast session-start hook launches workers. If gateway startup raises or is interrupted, this hook has already returned, so its exception handler cannot clean up. Pytest skips pytest_sessionfinish when session startup fails. The database and uploaded objects remain; the sweeper eventually removes only metadata.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Repair in 48a2fcf: verified in _pytest/main.py, which calls pytest_sessionfinish only when initstate >= 2. config.add_cleanup, by contrast, runs in _ensure_unconfigure through an ExitStack.

Each removal is now registered as a config cleanup before the step it undoes:

  • the fixture schema (DROP DATABASE IF EXISTS plus deleting the data objects, both idempotent), before it is created;
  • the worker schema's DROP, and the S3 Tables namespace deletion right after the namespace exists.

pytest_sessionfinish keeps only the guard, which runs before these cleanups.

Checked against AWS with a plugin that raises in a trylast pytest_sessionstart after this hook: the run aborts, and both schemas it created are gone (Glue get_database returns EntityNotFoundException for both recorded names). A normal -n 2 run also removes the controller's fixture schema and both workers' schemas.

.paginate(DatabaseName=ENV.fixture_schema)
for table in page["TableList"]
}
except Exception as e:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent review (relayed Codex result), finding 2 of 3

P2 — Deleting the fixture database bypasses the guard. If erroneous test cleanup drops the fixture database after its last reader finishes, Glue's get_tables raises EntityNotFoundException. This handler prints a warning and returns without changing the successful exit status. The subsequent DROP DATABASE IF EXISTS succeeds, leaving the run green despite every fixture table being missing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Repair in 48a2fcf: verified. The guard now treats EntityNotFoundException as an empty table set, so a dropped fixture schema reports every table as missing and fails the run. Any other listing error is still only reported. A temporary test (not committed) that dropped the fixture schema produced missing: ['boolean_na_values', ..., 'view_one_row'] and exit status 1.

Comment thread docs/testing.md Outdated

The tables and views from `tests/pyathena/tables.py` and the data files they read are in a fixture schema, `ENV.fixture_schema`, which tests only read.
With pytest-xdist, the controller creates it once before the workers start and drops it after they finish.
The controller loads these session hooks only when a path given to pytest is `tests/pyathena/` or inside it; otherwise, and in a run without workers, each test process creates and drops its own fixture schema.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent review (relayed Codex result), finding 3 of 3

P3 — The documented fallback for broader test paths does not run. For pytest -n 8 tests/ --ignore=tests/sqlalchemy, this nested conftest loads during collection in workers, after pytest_sessionstart. Pytest replays pytest_configure, but does not replay session startup, so neither schema is created and integration tests encounter missing resources. The newly documented fallback is therefore inaccurate.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Repair in 48a2fcf: verified. Workers get the same arguments and rootdir as the controller, so they load the same initial conftests, and the fallback could never run. The code now treats any process without workerinput as the owner, and a worker takes the controller's name directly. The testing guide replaces the fallback sentence: a run without workers creates the fixture schema itself, and the session hooks run only when a path given to pytest is tests/pyathena/ or inside it, as with just test pyathena. markdownlint-cli2 reports 0 errors.

Comment thread tests/pyathena/conftest.py Outdated
if _is_test_process(config):
_create_s3tables_namespace()
config.add_cleanup(_delete_s3tables_namespace)
config.add_cleanup(functools.partial(_drop_database, ENV.schema))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent follow-up review (relayed Codex result): FINDINGS (2 × P2)

Reviewer: Codex CLI 0.157.1, model gpt-6-astra, session 01a0e811-e6fa-7491-b555-c3551fd22015. The review ran read-only on a detached snapshot at 48a2fcf and covered 01b24f4..48a2fcf. It was a static review only. The reviewer confirmed that all three earlier findings are resolved, that the guard runs before the drops, and that the database is dropped before the namespace is deleted.

P2 — Worker cleanup runs after xdist announces completion. xdist sends workerfinished when pytest_sessionfinish completes, before config cleanups execute. Once all workers announce completion, the controller terminates their gateways with a 10-second timeout. A worker still waiting for an Athena database drop can be interrupted, leaving its schema or namespace behind. Cleanup exceptions can also leave the run green.

P2 — Namespace cleanup is registered after creation. If S3 Tables creates the namespace but the client raises while receiving the response, execution never reaches add_cleanup(_delete_s3tables_namespace), and the namespace leaks. Register namespace removal before calling _create_s3tables_namespace().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Repair in e66cdce. I checked both findings against the code.

  1. xdist/remote.py WorkerInteractor.pytest_sessionfinish is a hookwrapper that sends workerfinished after yield, so it runs after this conftest's hook. The removals now happen in pytest_sessionfinish again: guard first, then _run_cleanups, in finally. They are recorded in _cleanups before each create step. A config cleanup (_run_cleanups) runs only what is still recorded, which covers a failed session start where pytest skips pytest_sessionfinish. Each run clears the list, so nothing runs twice.
  2. _delete_s3tables_namespace is recorded before _create_s3tables_namespace(). It now does nothing when get_namespace raises NotFoundException.

Rechecked against AWS, including S3 Tables namespaces (catalog configured):

  • -n 2: 11 passed, and no recorded database or namespace was left.
  • A failure injected after this hook's session start: nothing was left.
  • The fixture schema dropped by a test: the guard reported every table missing, the run exited 1, and nothing was left.

session: The pytest session.
"""
config = session.config
config.add_cleanup(_run_cleanups)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent follow-up review 2 (relayed Codex result): FINDINGS (2 × P2)

  • Reviewer: Codex CLI 0.157.1, model gpt-6-astra, session 01a0e818-036e-7013-9584-87fb5152b3fc.
  • Scope: read-only, detached snapshot at e66cdce, range 48a2fcf..e66cdce. Static review only.
  • Confirmed: normal worker cleanup now precedes workerfinished, and draining the list prevents duplicate cleanup. Namespace deletion is now registered before creation.

P2 — Failed worker startup can still lose cleanup. Take -n 1 with a namespace that is created successfully and a database creation that raises. Pytest reports pytest_internalerror before reaching the config cleanup. xdist then sends internal_error and removes the worker. The controller terminates the gateways without waiting, so a slow database removal can be killed, and the namespace removal never runs. Cleanup must complete before the startup exception reaches xdist's error notification.

P2 — New namespace lookup requires an undocumented permission and blocks deletion. An identity that has the S3 Tables permissions listed in docs/testing.md:97 lacks s3tables:GetNamespace. The lookup raises AccessDeniedException before any deletion. Because the list was already cleared, the fallback cannot retry. Add the permission to the documentation, or detect absence with an operation the tests already require.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Repair in b127cc9: both findings verified.

  1. pytest_sessionstart again wraps its steps in try/except BaseException. On failure it runs the recorded removals immediately, under suppress(Exception), and re-raises the original error. The removals therefore finish before the error reaches xdist. The config cleanup stays for failures in later session-start hooks, such as xdist starting the workers on the controller.
  2. GetNamespace is gone. _delete_s3tables_namespace now detects a missing namespace by catching NotFoundException from ListTables, which the cleanup already used. I checked against AWS that ListTables on a nonexistent namespace raises NotFoundException.

Checked against AWS with a helper plugin:

  • -n 1, with the worker's _create_database patched to raise after the namespace was created: the run failed with the simulated error, and the worker's schema, its namespace, and the controller's fixture schema were all gone.
  • A normal -n 2 run: 11 passed, with nothing left.

_cleanups.append(functools.partial(_drop_database, ENV.schema))
with contextlib.closing(connect()) as conn, conn.cursor() as cursor:
_create_database(cursor, ENV.schema)
except BaseException:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent follow-up review 3 (relayed Codex result): FINDINGS (1 × P2)

  • Reviewer: Codex CLI 0.157.1, model gpt-6-astra, session 01a0e820-245e-7fa0-a54d-77fd02835cd8.
  • Scope: read-only, detached snapshot at b127cc9, covering e66cdce..b127cc9. Static only.
  • Confirmed: both earlier findings are resolved. Clearing _cleanups before execution prevents duplicate removals across the immediate, session-finish, and config-cleanup paths.

P2 — Later worker startup failures still race gateway termination. Scenario with -n 1: this hook creates the worker's resources, then another plugin's trylast=True pytest_sessionstart raises. This local except cannot catch that error. pytest reports internal_error before the config cleanup runs and skips pytest_sessionfinish. xdist then removes the worker and proceeds to gateway termination, which can prevent the namespace removal. Cleanup must surround the entire startup-hook invocation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Repair in 1e2bc72: verified.

pytest_sessionstart is now a new-style wrapper (@pytest.hookimpl(wrapper=True)). It creates the schemas before yield. On the controller, that still happens before xdist's session start launches the workers. try/except BaseException surrounds both the creation and yield, so a failure in this hook or in any later session-start hook runs the recorded removals before the error propagates to xdist. Because this wrapper covers every path that skips pytest_sessionfinish, the config-cleanup fallback is removed.

AWS results with a helper plugin, checking every recorded name, and also every pyathena_test_ database and namespace created in the last 10 minutes:

  • A, normal -n 2: passed, nothing left.
  • B, -p no:xdist, a trylast session-start hook raising after this one: the run aborted, nothing left.
  • D, -n 1, the worker's database creation raising after its namespace was created: the run aborted, nothing left.
  • E, -n 1, a trylast session-start hook raising on the worker after this wrapper created the worker's schema and namespace (the reported scenario): the run aborted, nothing left.

_cleanups.append(functools.partial(_drop_database, ENV.schema))
with contextlib.closing(connect()) as conn, conn.cursor() as cursor:
_create_database(cursor, ENV.schema)
return (yield)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent follow-up review 4 (relayed Codex result): FINDINGS (1 × P2)

Reviewer: Codex CLI 0.157.1, model gpt-6-astra, session 01a0e829-f367-7a32-8e93-2e6c661df59f. Read-only review of a detached snapshot at 1e2bc72, covering b127cc9..1e2bc72, against the local pytest 9.0.3, pytest-xdist 3.6.1, and Pluggy sources. Static review only.

Confirmed:

  • The earlier trylast finding is resolved.
  • The controller still creates the fixture schema before the workers launch.
  • Removals run once on normal completion, on failures this wrapper catches, and on interrupts.

P2 — Outer session-start wrapper failures bypass cleanup. A plugin implementing pytest_sessionstart with wrapper=True, tryfirst=True can yield, then raise or receive KeyboardInterrupt during its post-yield work. At that point Pluggy has already completed this conftest's wrapper, so the wrapper's except never runs, and pytest skips pytest_sessionfinish. With the config-cleanup fallback removed, the recorded removals never run. For controller and non-worker runs this regresses the former fallback behavior.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Repair in d48df7f: verified. The wrapper again registers config.add_cleanup(_run_cleanups) before it creates anything. _run_cleanups clears the recorded list before running it, so the fallback is a no-op after a normal pytest_sessionfinish or after the wrapper's own failure path. It runs the removals only when a failure outside this wrapper skips pytest_sessionfinish, for example in an outer tryfirst wrapper.

Limit: no plugin that this project installs wraps pytest_sessionstart in that way. On a worker, such a failure would still race xdist's gateway termination. That residual case is accepted here rather than modeled further.

Checked against AWS: a normal -n 2 run left nothing, and a run with -p no:xdist and a failing trylast session-start hook left nothing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent follow-up review 5 (relayed Codex result): CLEAN

  • Reviewer: Codex CLI 0.157.1, model gpt-6-astra, session 01a0e82d-a30f-7360-b830-2d9dfd9f8c9f.
  • Scope: read-only review of a detached snapshot at d48df7f, covering 1e2bc72..d48df7f. The snapshot was unchanged afterwards. This was a static review only.
  • Checked:
    • normal finish
    • partial creation
    • failures in the inner and outer session-start hooks
    • cleanup exceptions
  • Result:
    • The prior finding is resolved for the controller and for runs without workers.
    • Each recorded removal is attempted exactly once. The list is cleared before the removals run, so later cleanup calls do nothing, even after a removal fails.
    • "No new concrete defects found."

@laughingman7743
laughingman7743 marked this pull request as ready for review September 28, 2026 13:25
@laughingman7743
laughingman7743 force-pushed the test/848-shared-fixture-schema branch from d48df7f to 1f31ee6 Compare September 28, 2026 15:01
@laughingman7743
laughingman7743 force-pushed the test/848-shared-fixture-schema branch from 1f31ee6 to c431822 Compare September 28, 2026 15:01
@laughingman7743
laughingman7743 force-pushed the test/848-shared-fixture-schema branch from c431822 to 3145c86 Compare September 28, 2026 15:07
@laughingman7743
laughingman7743 force-pushed the test/848-shared-fixture-schema branch from 3145c86 to adb1295 Compare September 28, 2026 15:13
@laughingman7743
laughingman7743 force-pushed the test/848-shared-fixture-schema branch from adb1295 to 32603c2 Compare September 28, 2026 16:02
laughingman7743 and others added 6 commits September 29, 2026 01:02
The read-only tables and views from tests/pyathena/tables.py, their data
files, the Spark CSV, and the filesystem test file now live in one fixture
schema (ENV.fixture_schema) per pytest run instead of in every worker's
schema. The pytest-xdist controller creates it before the workers start,
passes its name to them through workerinput, and drops it after they
finish; a run without workers creates it itself. Each test process still
creates its own ENV.schema and S3 Tables namespace for the objects its
tests create.

The cursor and engine fixtures default to the fixture schema. Reads that
named ENV.schema for fixture tables now name ENV.fixture_schema, and the
pandas to_sql tests read their tables back through ENV.schema. Before
dropping the fixture schema, its owner lists its tables and fails the run
if they differ from the ones it created.

With -n 8, session setup and teardown drop from 88 Athena statements to 27.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e schema

The schemas are dropped by config cleanups registered before each create
step, which pytest runs even when a later session-start hook, such as
pytest-xdist starting its workers, fails and pytest_sessionfinish is
skipped. pytest_sessionfinish only runs the fixture-schema check, which now
treats a missing fixture schema as holding no tables. The worker fallback
for a controller without this conftest is removed: workers load the same
initial conftests, so the testing guide states that the hooks run only for
paths under tests/pyathena/.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A pytest-xdist worker reports that it finished after pytest_sessionfinish,
and the controller may then stop it, so the removals run in
pytest_sessionfinish again. They are recorded before each create step, and a
config cleanup runs whatever is still recorded when pytest skipped
pytest_sessionfinish after a failed session start. Deleting the S3 Tables
namespace is recorded before creating it and does nothing if it does not
exist.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A failure in this conftest's session start runs the recorded removals before
the error reaches pytest-xdist, which may stop a worker that reports it; the
config cleanup still covers failures in later session-start hooks. The S3
Tables namespace deletion detects a missing namespace through ListTables,
which the tests already need, instead of GetNamespace.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pytest_sessionstart is now a wrapper: it creates the schemas before the other
session-start hooks and runs the recorded removals when this or a later
session-start hook fails, before the error reaches pytest-xdist. The config
cleanup fallback is no longer needed and is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
engine, conn = engine
one_row_complex = Table("one_row_complex", MetaData(schema=ENV.schema), autoload_with=conn)
one_row_complex = Table(
"one_row_complex", MetaData(schema=ENV.fixture_schema), autoload_with=conn

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent review of the rebase (relayed Codex result): CLEAN

  • Reviewer: Codex CLI 0.157.1, model gpt-6-astra, session 01a0e8c4-1989-7da0-b3bc-d5bd1d02e76f. The review ran read-only on a detached snapshot at 9d34a4b and was static only.
  • Rebase reviewed: git range-diff c3b08bb..adb1295 a86a180..9d34a4b. The series moved off the closed Derive the one_row_complex expectations of the Python-object cursors #875–Add an ARRAY<string> column to one_row_complex through its definition only #877 onto master, which keeps the hand-written expectations.
  • Reported:
    • Five patches are unchanged. The first differs only at the test_reflect_select conflict, which keeps the hand-written assertions and reads from ENV.fixture_schema.
    • All shared tables and views, the Spark CSV, and the filesystem test file are read from ENV.fixture_schema, from its S3 prefix, or unqualified through the fixtures' default schema. No test writes into the fixture schema.
    • Setup, worker schema propagation, teardown, and failure cleanup show no regression.
    • No references to the dropped expected.py helpers remain.
  • Local checks:

@laughingman7743

Copy link
Copy Markdown
Member Author

Closing without merging, by the maintainer's decision. Per pytest -n 8 job, the shared fixture schema saves 61 setup statements (88 to 27) out of a few thousand queries. Athena does not charge for DDL, so the saving comes to under a tenth of a cent per job, and the wall time is about the same. It needs a considerably more complex session lifecycle to get there: xdist hooks, workerinput, a session-start wrapper with cleanup fallbacks, and a fixture-schema guard. It also changes the default schema, so tests must keep telling the fixture schema from ENV.schema. #866 and #873 already reduced the setup from 333 to 88 statements per job. The branch is kept for reference.

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