Skip to content

SNOW-3225365: Session-scoped tmp_schema and TEMPORARY tables in e2e tests#604

Merged
sfc-gh-fpawlowski merged 12 commits intomainfrom
SNOW-3225365/tmp-schema-perf
Mar 24, 2026
Merged

SNOW-3225365: Session-scoped tmp_schema and TEMPORARY tables in e2e tests#604
sfc-gh-fpawlowski merged 12 commits intomainfrom
SNOW-3225365/tmp-schema-perf

Conversation

@sfc-gh-fpawlowski
Copy link
Contributor

Summary

  • tmp_schema is now session-scoped — previously function-scoped, it executed CREATE SCHEMA + DROP SCHEMA for every test (2 round-trips × N tests). Now a single schema is created once at session start and dropped via DROP SCHEMA IF EXISTS … CASCADE at session end.
  • Uses a dedicated connection from connection_factory (already session-scoped) so schema lifecycle does not compete with test queries and avoids resource contention.
  • All infrastructure CREATE TABLE calls changed to CREATE OR REPLACE TEMPORARY TABLE — necessary because multiple tests reuse the same table names (e.g. boolean_table) in the shared schema. CREATE OR REPLACE prevents conflicts; TEMPORARY auto-drops at session end with no manual cleanup.

Exceptions

  • test_should_execute_create_and_drop_table_statements — intentionally tests the CREATE TABLE + DROP TABLE DDL flow; left as plain CREATE TABLE.

Dependencies

Test plan

  • pytest python/tests/e2e/ -x -q --no-header — full e2e suite passes
  • pytest python/tests/integ/ -x -q — integ tests unaffected (use per-test connection directly)
  • Pre-commit hooks pass (tests-format-validator, ruff, mypy)
  • Timing improvement: time pytest python/tests/e2e/ --no-header -q --tb=no

🤖 Generated with Claude Code

…pass

Every test was executing a SELECT 1 round-trip via assert_connection_is_open
before its actual logic, wasting one network call per test.

Replace all 23 call sites with a no-op pass statement so the "Given Snowflake
client is logged in" BDD step still has an implementation (required by the
tests-format-validator) without making a redundant network round-trip.
Clean up the now-unused assert_connection_is_open imports.

The function definition in utils.py is kept intact.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sfc-gh-fpawlowski sfc-gh-fpawlowski requested a review from a team as a code owner March 18, 2026 23:06
Copilot AI review requested due to automatic review settings March 18, 2026 23:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes the Python e2e test suite’s Snowflake object lifecycle by reusing a single temporary schema for the test session and switching most test tables to CREATE OR REPLACE TEMPORARY TABLE, reducing per-test DDL round-trips and cleanup burden.

Changes:

  • Make tmp_schema session-scoped and create/drop it once per test session (via a dedicated connection).
  • Convert many e2e test tables to CREATE OR REPLACE TEMPORARY TABLE to avoid name conflicts and reduce cleanup needs.
  • Replace assert_connection_is_open(...) calls in many tests with a no-op (pass) and remove the corresponding imports.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
python/tests/conftest.py Changes tmp_schema fixture to session scope and manages schema lifecycle with a dedicated connection.
python/tests/e2e/types/test_timestamp_ntz.py Removes redundant connection-open assertions and uses temporary tables for schema-scoped table creation.
python/tests/e2e/types/test_timestamp_ltz.py Removes redundant connection-open assertions and uses temporary tables for schema-scoped table creation.
python/tests/e2e/types/test_string_lob.py Removes redundant connection-open assertions and uses temporary tables for LOB test tables.
python/tests/e2e/types/test_string.py Removes redundant connection-open assertions and uses temporary tables for table-based string tests.
python/tests/e2e/types/test_number_numpy.py Removes redundant connection-open assertions (tests are skipped) for NumPy number tests.
python/tests/e2e/types/test_number.py Removes redundant connection-open assertions and uses temporary tables for table-based number tests.
python/tests/e2e/types/test_int.py Removes redundant connection-open assertions and uses temporary tables for table-based int tests.
python/tests/e2e/types/test_float_numpy.py Removes redundant connection-open assertions (tests are skipped) for NumPy float tests.
python/tests/e2e/types/test_float.py Removes redundant connection-open assertions and uses temporary tables for table-based float tests.
python/tests/e2e/types/test_decfloat_numpy.py Removes redundant connection-open assertions (tests are skipped) for NumPy decfloat tests.
python/tests/e2e/types/test_decfloat.py Removes redundant connection-open assertions and uses temporary tables for table-based decfloat tests.
python/tests/e2e/types/test_boolean_numpy.py Removes redundant connection-open assertions (tests are skipped) for NumPy boolean tests.
python/tests/e2e/types/test_boolean.py Removes redundant connection-open assertions and uses temporary tables for table-based boolean tests.
python/tests/e2e/types/test_binary_lob.py Removes redundant connection-open assertions and uses temporary tables for binary LOB test tables.
python/tests/e2e/types/test_binary.py Removes redundant connection-open assertions and uses temporary tables for table-based binary tests.
python/tests/e2e/tls/test_crl_enabled.py Removes redundant connection-open assertion in CRL test.
python/tests/e2e/query/test_parameter_binding.py Removes redundant connection-open assertions and uses temporary tables for DML/DDL binding tests.
python/tests/e2e/query/test_large_result_set.py Removes redundant connection-open assertion in large result-set test.
python/tests/e2e/query/test_client_side_binding.py Removes redundant connection-open assertions and uses temporary tables for client-side binding tests.
python/tests/e2e/query/test_basic_execute_query.py Removes redundant connection-open assertions and uses temporary tables for DML setup (DDL test remains plain CREATE TABLE).
python/tests/e2e/put_get/test_put_get_source_compression.py Removes redundant connection-open assertions in PUT/GET compression tests.
python/tests/e2e/put_get/test_put_get_basic_operations.py Removes redundant connection-open assertions in PUT/GET basic operations tests.
python/tests/e2e/put_get/test_put_get_auto_compress.py Removes redundant connection-open assertions in PUT/GET auto-compress tests.

Follow-up to the assert_connection_is_open removal:

- put_get and tls tests: move "# Given Snowflake client is logged in"
  above the `with connection.cursor()` / `with connection_factory()` block
  so the step has a real implementation (the with-statement) rather than
  a no-op pass. Also drop the now-unused execute_query fixture parameter
  from these test functions.

- class-based type and query tests: remove the "# Given Snowflake client
  is logged in" comment and pass entirely. The connection is implicitly
  established by the execute_query/cursor fixtures; a separate BDD step
  for it adds noise without value.

- feature files: remove the "Given Snowflake client is logged in" step
  from Python-specific and shared feature files where the Python tests
  no longer implement it. Keeps the step in put_get/tls feature files
  where the with-block still serves as the implementation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sfc-gh-fpawlowski sfc-gh-fpawlowski force-pushed the SNOW-3225365/tmp-schema-perf branch from 520046f to 080d897 Compare March 18, 2026 23:38
…eature files

Replace every `pass` placeholder (used as Given step filler) with real
implementation code:
- Simple query tests: extract SQL string into `sql = "..."` variable
- Table tests: hoist `table_name = f"{tmp_schema}.xxx"` before the And step
- Parametrize tests (sql is a fixture param): use `assert callable(execute_query)`
- Multiline SQL expressions collected in full via paren-depth tracking

Restore all .feature files to main state, reverting the incorrect removals
of Given step lines introduced in the previous commit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 19, 2026 10:31
@sfc-gh-fpawlowski sfc-gh-fpawlowski force-pushed the SNOW-3225365/tmp-schema-perf branch from 080d897 to 2d81b10 Compare March 19, 2026 10:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes the Python e2e test suite setup/teardown by making tmp_schema session-scoped (created once per test session and dropped once at the end) and by switching test-created tables to CREATE OR REPLACE TEMPORARY TABLE to avoid cross-test conflicts in the shared schema. It also removes the per-test “connection open” probe (SELECT 1) and does small refactors to keep the BDD-style comments intact while reducing redundant work.

Changes:

  • Add an e2e-local, session-scoped tmp_schema fixture backed by a dedicated connection, dropped with DROP SCHEMA IF EXISTS … CASCADE.
  • Replace many CREATE TABLE statements in e2e tests with CREATE OR REPLACE TEMPORARY TABLE to avoid name collisions and cleanup overhead.
  • Remove assert_connection_is_open call sites and adjust tests to inline SQL strings / table names instead.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
python/tests/e2e/conftest.py Adds session-scoped tmp_schema fixture for e2e tests using a dedicated connection.
python/tests/e2e/types/test_timestamp_ntz.py Removes connection probe; uses session schema and temporary tables for table-backed tests.
python/tests/e2e/types/test_timestamp_ltz.py Removes connection probe; uses session schema and temporary tables for table-backed tests.
python/tests/e2e/types/test_string_lob.py Removes connection probe; uses temporary tables for LOB tests.
python/tests/e2e/types/test_string.py Removes connection probe; uses temporary tables; minor refactor around corner-case queries.
python/tests/e2e/types/test_number_numpy.py Removes connection probe; inlines SQL strings for cursor-based execution.
python/tests/e2e/types/test_number.py Removes connection probe; uses temporary tables for table-backed number tests.
python/tests/e2e/types/test_int.py Removes connection probe; uses temporary tables for table-backed int tests.
python/tests/e2e/types/test_float_numpy.py Removes connection probe; inlines SQL strings for cursor-based execution.
python/tests/e2e/types/test_float.py Removes connection probe; uses temporary tables for table-backed float tests.
python/tests/e2e/types/test_decfloat_numpy.py Removes connection probe; inlines SQL strings for cursor-based execution.
python/tests/e2e/types/test_decfloat.py Removes connection probe; uses temporary tables for table-backed decfloat tests.
python/tests/e2e/types/test_boolean_numpy.py Removes connection probe; inlines SQL strings for cursor-based execution.
python/tests/e2e/types/test_boolean.py Removes connection probe; uses temporary tables for table-backed boolean tests.
python/tests/e2e/types/test_binary_lob.py Removes connection probe; uses temporary tables for binary LOB tests.
python/tests/e2e/types/test_binary.py Removes connection probe; uses temporary tables for table-backed binary tests.
python/tests/e2e/tls/test_crl_enabled.py Removes connection probe and unused fixture arg; relies on connection_factory for the CRL-enabled connection.
python/tests/e2e/query/test_parameter_binding.py Removes connection probe; uses temporary tables for DML cases; minor reshuffling.
python/tests/e2e/query/test_large_result_set.py Removes connection probe; keeps large-result-set query deterministic via ROW_NUMBER().
python/tests/e2e/query/test_client_side_binding.py Removes connection probe; uses temporary tables; reshuffles “Given” lines.
python/tests/e2e/query/test_basic_execute_query.py Removes connection probe; uses temporary tables; reshuffles “Given” lines.
python/tests/e2e/put_get/test_put_get_source_compression.py Removes connection probe and unused fixture arg; uses connection directly.
python/tests/e2e/put_get/test_put_get_basic_operations.py Removes connection probe and unused fixture arg; uses connection directly.
python/tests/e2e/put_get/test_put_get_auto_compress.py Removes connection probe and unused fixture arg; uses connection directly.

Comment on lines 22 to 25
def test_should_bind_basic_types_with_positional_pyformat(self, execute_query, cursor):
"""Test basic type binding with %s placeholders."""
# Given Snowflake client is logged in with pyformat paramstyle
assert_connection_is_open(execute_query)
"""Test basic type binding with %s placeholders."""

…e was misplaced

In 8 test methods, code was hoisted to the Given step even though a
method-level explanatory comment appeared above it in the original. This
left the comment stranded after the code it described, making it
contextually wrong.

Restore the correct order: explanatory comment before Given, `pass` as
the Given implementation (there is no meaningful code to hoist that
would not displace the comment), and the code back in its original step.

Affected methods:
- test_binary.py: test_should_select_binary_with_specified_length_from_table,
  test_should_select_binary_literals_using_parameter_binding
- test_binary_lob.py: both LOB size limit tests
- test_string.py: corner case literals test, parameter binding test
- test_string_lob.py: both LOB size limit tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sfc-gh-fpawlowski sfc-gh-fpawlowski force-pushed the SNOW-3225365/tmp-schema-perf branch from 2d81b10 to edecd6e Compare March 20, 2026 11:36
… in Given

In test_basic_execute_query.py, test_client_side_binding.py,
test_parameter_binding.py and test_decfloat.py the transformer had
hoisted the method docstring to serve as the Given step implementation,
leaving it after the # Given comment instead of as the first statement.

Restore the docstrings to their correct first-statement position and use
pass as the Given implementation, consistent with other methods where
there is no meaningful code to hoist without displacing context.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 20, 2026 14:32
@sfc-gh-fpawlowski sfc-gh-fpawlowski force-pushed the SNOW-3225365/tmp-schema-perf branch from edecd6e to 2fa1e1b Compare March 20, 2026 14:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Python e2e test infrastructure to reduce per-test DDL overhead by switching tmp_schema to a session-scoped fixture and moving most test tables to CREATE OR REPLACE TEMPORARY TABLE, alongside removing redundant “connection is open” round-trips.

Changes:

  • Add python/tests/e2e/conftest.py to override tmp_schema as session-scoped (create once, drop once with CASCADE).
  • Convert many e2e table setups to CREATE OR REPLACE TEMPORARY TABLE to avoid name conflicts under the shared schema.
  • Remove assert_connection_is_open usage across e2e tests (replacing with pass or reorganizing SQL strings) and simplify some test signatures that no longer need execute_query.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
python/tests/e2e/conftest.py Introduces session-scoped tmp_schema for e2e runs and session-end schema teardown.
python/tests/e2e/types/test_timestamp_ntz.py Drops connection-open assertion and switches table creation to temporary tables.
python/tests/e2e/types/test_timestamp_ltz.py Drops connection-open assertion and switches table creation to temporary tables.
python/tests/e2e/types/test_string_lob.py Removes connection-open assertion; uses temporary tables for LOB tests.
python/tests/e2e/types/test_string.py Removes connection-open assertion; uses temporary tables for table-based string tests.
python/tests/e2e/types/test_number_numpy.py Removes connection-open assertion and minor SQL variable refactors.
python/tests/e2e/types/test_number.py Removes connection-open assertion; uses temporary tables for table-based number tests.
python/tests/e2e/types/test_int.py Removes connection-open assertion; uses temporary tables for table-based int tests.
python/tests/e2e/types/test_float_numpy.py Removes connection-open assertion and minor SQL variable refactors.
python/tests/e2e/types/test_float.py Removes connection-open assertion; uses temporary tables for table-based float tests.
python/tests/e2e/types/test_decfloat_numpy.py Removes connection-open assertion and minor SQL variable refactors.
python/tests/e2e/types/test_decfloat.py Removes connection-open assertion; uses temporary tables for table-based decfloat tests.
python/tests/e2e/types/test_boolean_numpy.py Removes connection-open assertion and minor SQL variable refactors.
python/tests/e2e/types/test_boolean.py Removes connection-open assertion; uses temporary tables for table-based boolean tests.
python/tests/e2e/types/test_binary_lob.py Removes connection-open assertion; uses temporary tables for binary LOB tests.
python/tests/e2e/types/test_binary.py Removes connection-open assertion; uses temporary tables for table-based binary tests.
python/tests/e2e/tls/test_crl_enabled.py Removes redundant connection-open check and simplifies test signature.
python/tests/e2e/query/test_parameter_binding.py Removes redundant connection-open check; uses temporary tables for binding tests.
python/tests/e2e/query/test_large_result_set.py Removes redundant connection-open check; minor SQL refactor.
python/tests/e2e/query/test_client_side_binding.py Removes redundant connection-open checks; uses temporary tables for pyformat tests.
python/tests/e2e/query/test_basic_execute_query.py Removes redundant connection-open checks; uses temp tables for DML test setup.
python/tests/e2e/put_get/test_put_get_source_compression.py Removes redundant connection-open check; simplifies signatures to just connection.
python/tests/e2e/put_get/test_put_get_basic_operations.py Removes redundant connection-open check; simplifies signatures to just connection.
python/tests/e2e/put_get/test_put_get_auto_compress.py Removes redundant connection-open check; simplifies signatures to just connection.
Comments suppressed due to low confidence (1)

python/tests/e2e/query/test_basic_execute_query.py:123

  • With a session-scoped tmp_schema, a failure between CREATE TABLE and the final DROP TABLE can leave test_basic_ddl behind in the shared schema and potentially break later tests in the same run. Wrap the create/assertions in a try/finally and ensure cleanup with DROP TABLE IF EXISTS ... in the finally block (keeping the plain CREATE TABLE as intended for this test).
        table_name = f"{tmp_schema}.test_basic_ddl"

        # When CREATE TABLE statement is executed
        cursor.execute(f"CREATE TABLE {table_name} (id NUMBER, name VARCHAR)")

        # Then the table should be created successfully
        cursor.execute(f"SELECT COUNT(*) FROM {table_name}")
        result = cursor.fetchone()
        assert result[0] == 0

        # And DROP TABLE statement should complete successfully
        cursor.execute(f"DROP TABLE {table_name}")
        result = cursor.fetchone()

…ll step comment

Variables hoisted to Given implementations were left before step comments.
Move sql= / table_name= back to after all continuation lines of their step,
and use pass as the honest Given implementation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sfc-gh-fpawlowski sfc-gh-fpawlowski force-pushed the SNOW-3225365/tmp-schema-perf branch from 2fa1e1b to d94af33 Compare March 20, 2026 14:59
Two methods had sql= hoisted under # Given with a method-level comment
placed after it, which the de-hoisting script missed. Restore the
method-level comment to before Given and move sql= to after When.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.

sfc-gh-fpawlowski and others added 3 commits March 20, 2026 16:25
The commits following the assert_connection_is_open removal introduced
structural changes (docstring removal, Note: comment reordering, variable
hoisting, Given step restructuring) that are outside this PR's scope.

Restore all affected files to the state they were in immediately after
the assert_connection_is_open removal commit (adf4926).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… removal

Removing assert_connection_is_open(execute_query) left execute_query
declared in 95 test method signatures but never referenced in their bodies.
These are the put_get, tls, query and numpy-type tests that operate
directly on cursor/connection_factory/cursor_with_numpy fixtures.

Remove the dead parameter from all 95 affected signatures.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sfc-gh-fpawlowski sfc-gh-fpawlowski force-pushed the SNOW-3225365/tmp-schema-perf branch from fb328a5 to 4017a94 Compare March 23, 2026 09:34
Conflict resolutions:
- python/tests/e2e/types/test_float.py: take pass under Given (main),
  restore columns = ... under When (dropped by merge)
- python/tests/e2e/types/test_binary_lob.py: keep blank line after pass
  (main style)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Reword docstring: "must use TEMPORARY TABLE" → "should generally use";
  clarify lifecycle (dropped when the creating connection closes, not at
  pytest session end); note intentional non-temporary DDL tests
- Use a fresh connection for DROP SCHEMA in teardown so cleanup does not
  depend on the long-lived session connection (prevents orphaned schemas
  if the connection times out mid-run)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 24, 2026 07:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

@sfc-gh-fpawlowski sfc-gh-fpawlowski added this pull request to the merge queue Mar 24, 2026
@sfc-gh-fpawlowski sfc-gh-fpawlowski self-assigned this Mar 24, 2026
Merged via the queue into main with commit 9ea2e70 Mar 24, 2026
62 of 64 checks passed
@sfc-gh-fpawlowski sfc-gh-fpawlowski deleted the SNOW-3225365/tmp-schema-perf branch March 24, 2026 09:13
sfc-gh-turbaszek pushed a commit that referenced this pull request Mar 25, 2026
…ests (#604)

## Summary

- **`tmp_schema` is now session-scoped** — previously function-scoped,
it executed `CREATE SCHEMA` + `DROP SCHEMA` for every test (2
round-trips × N tests). Now a single schema is created once at session
start and dropped via `DROP SCHEMA IF EXISTS … CASCADE` at session end.
- Uses a **dedicated connection** from `connection_factory` (already
session-scoped) so schema lifecycle does not compete with test queries
and avoids resource contention.
- **All infrastructure `CREATE TABLE` calls changed to `CREATE OR
REPLACE TEMPORARY TABLE`** — necessary because multiple tests reuse the
same table names (e.g. `boolean_table`) in the shared schema. `CREATE OR
REPLACE` prevents conflicts; `TEMPORARY` auto-drops at session end with
no manual cleanup.

## Exceptions

- `test_should_execute_create_and_drop_table_statements` — intentionally
tests the `CREATE TABLE` + `DROP TABLE` DDL flow; left as plain `CREATE
TABLE`.

## Dependencies

- Stacks on top of #603 (`SNOW-3225365/remove-assert-connection`); both
target `main` independently and can be merged in any order.

## Test plan

- [ ] `pytest python/tests/e2e/ -x -q --no-header` — full e2e suite
passes
- [ ] `pytest python/tests/integ/ -x -q` — integ tests unaffected (use
per-test `connection` directly)
- [ ] Pre-commit hooks pass (`tests-format-validator`, `ruff`, `mypy`)
- [ ] Timing improvement: `time pytest python/tests/e2e/ --no-header -q
--tb=no`

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

3 participants