From 516bef18b5ebd5f9289fd31e43ed24aa12a484ad Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sun, 4 Oct 2026 17:29:44 +0900 Subject: [PATCH 1/6] test: audit conventions and clarify regression expectations --- AGENTS.md | 6 +- docs/testing-audit.md | 207 ++++++++++++++++ docs/testing.md | 26 ++ tests/pyathena/filesystem/test_s3.py | 22 +- tests/pyathena/filesystem/test_s3_async.py | 21 +- tests/pyathena/pandas/test_result_set.py | 275 +++++++++++++-------- tests/pyathena/polars/test_result_set.py | 7 +- 7 files changed, 453 insertions(+), 111 deletions(-) create mode 100644 docs/testing-audit.md diff --git a/AGENTS.md b/AGENTS.md index 51c300485..88b9fde94 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -82,7 +82,8 @@ uv run --env-file .env pytest -n 1 tests/pyathena/test_cursor.py -v #### Test Conventions - **Class-based tests** for integration tests that use fixtures (cursors, engines): `class TestCursor:` with methods like `def test_fetchone(self, cursor):` -- **Standalone functions** for unit tests of pure logic (converters, parsers, utils): `def test_to_struct_json_formats(input_value, expected):` +- **Standalone functions** are the default for stateless helpers: `def test_to_struct_json_formats(input_value, expected):`. Unit tests may use a class when it groups the behavior of one object or meaningful common setup, such as `TestTypeSignatureParser`. +- Function-oriented utility tests may remain standalone even when they use AWS fixtures, as in `tests/pyathena/pandas/test_util.py`. Fixture use alone does not determine the grouping. - Test file naming mirrors source: `pyathena/parser.py` → `tests/pyathena/test_parser.py` - **Fixtures**: Cursor/engine fixtures are defined in `conftest.py` and injected by name (e.g., `cursor`, `engine`, `async_cursor`). Use `indirect=True` parametrization to pass connection options: @@ -93,6 +94,9 @@ uv run --env-file .env pytest -n 1 tests/pyathena/test_cursor.py -v ``` - **Parametrize** with `@pytest.mark.parametrize(("input", "expected"), [...])` for data-driven tests +- Keep parameter definitions declarative. Build mocks, configured result sets, and one-shot readers during fixture setup or test execution. Pure values and framework type/expression objects may be constructed in parameter definitions. +- Make expected values independent of the implementation under test. A library comparison is appropriate when matching that library is the contract; describe intentional differences with explicit expected values. +- Preserve test IDs, parameter coverage, marks, fixture scopes, and resource cleanup when reorganizing tests. SQLAlchemy compliance tests retain their upstream class and plugin conventions and applicable attribution. - **Integration tests** (need AWS) use cursor/engine fixtures with real Athena queries; **unit tests** (no AWS) call functions directly with test data ### Markdown Lint diff --git a/docs/testing-audit.md b/docs/testing-audit.md new file mode 100644 index 000000000..b5669ef9d --- /dev/null +++ b/docs/testing-audit.md @@ -0,0 +1,207 @@ + + +# Test conventions audit + +This report records the source audit for [issue #1079](https://github.com/pyathena-dev/PyAthena/issues/1079). +The baseline is commit `200762088e7b0bac45054f22e32a16aa0ce95dbb`. +Source references below identify that baseline, so later edits do not change their meaning. +The accompanying changes clarify the conventions and address the findings listed here. + +## Reviewed areas + +The inventory covers all 83 tracked files under `tests/`: 80 Python files, two Jinja query templates, and the SQLAlchemy profiles file. +The Python inventory includes 17 package initializers and all three `conftest.py` files. +Structural inspection covered class and function organization, fixture arguments and decorators, parameter construction, helper placement, assertions, and synchronous and asynchronous counterparts. +Focused source tracing checked the setup and expected-value patterns identified by that inspection. +The complete file inventory appears below. + +| Area | Files | Conventions examined | +| --- | ---: | --- | +| Shared PyAthena tests and helpers | 15 | Cursor integration classes; standalone conversion helpers; object-oriented parser, model, result-set, and connection tests; session data and resource setup | +| Native asyncio tests and fixtures | 18 | Async cursor and dialect classes, async result-set helpers, per-test cursor contexts, connection teardown, and backend-specific cases | +| Arrow | 6 | Cursor classes, converter functions and classes, schema and precision assertions, mocked result-set setup | +| pandas | 7 | Cursor classes, function-oriented utility tests, reader lifetimes, CSV options and expected frames, dtype and option-context coverage | +| Polars | 5 | Cursor and converter tests, typed local files, failure after partial reads, and one-shot iterator inputs | +| S3FS cursors and readers | 4 | Cursor fixtures, custom converters, CSV NULL versus empty-string assertions, and reader context managers | +| Spark | 4 | Session and calculation fixtures, event-based concurrency helpers, cancellation assertions, and Future-based async behavior | +| Filesystem | 9 | Class fixtures, stubbed provider calls, path/value parametrization, sync/async operations, cleanup, and tests without observable assertions | +| PyAthena SQLAlchemy tests | 8 | Compiler and type unit tests, engine integration classes, local fixture reuse, SQL expression parameters, and reflected metadata assertions | +| SQLAlchemy compliance suite | 4 | Upstream inheritance, combinations and requirements, plugin hooks, sync/async selection, and explicit skips | +| Shared environment initializer | 1 | Required environment values and per-process schema identity | +| Query templates | 2 | Database setup/teardown, schema interpolation, and license notices | + +The compliance suite imports additional tests from the installed SQLAlchemy package. +Those upstream implementations and generated cases are outside the project-owned source inventory. +The audit does not establish complete runtime coverage or the correctness of every assertion. +Unmerged worktrees, including the pandas JSON changes associated with issue #1078, are outside this baseline. + +## Findings addressed + +The priorities describe coverage and maintenance impact, rather than product-defect severity. + +| Priority | Baseline evidence | Impact | Disposition | +| --- | --- | --- | --- | +| High | [tests/pyathena/filesystem/test_s3.py:4432][sync-du] and [tests/pyathena/filesystem/test_s3_async.py:1491][async-du] | Both `test_du` bodies contain only `pass`. A broken disk-usage implementation still produces two passing tests. | Replace both placeholders with real S3 file-size, total, depth, and single-file assertions. Preserve the names, use the existing filesystem fixtures, and remove the test objects in `finally`. | +| Medium | [tests/pyathena/pandas/test_result_set.py:265][csv-helper] and [tests/pyathena/pandas/test_result_set.py:305][csv-params] | Parameter evaluation repeatedly patches result-set properties and derives CSV options, including repeated default dtype construction. An option-building error prevents collection of unrelated tests in the module. | Store input metadata and options in the parameters. Build the options once per invocation, inside the pandas option context. Remove the unused base-initializer patch; `__new__` already bypasses `__init__`. | +| Medium | [tests/pyathena/pandas/test_result_set.py:391][csv-oracle] | The expectation reads the CSV with two engines and conditionally replaces columns and missing values. The reader has to reconstruct the intended contract from this algorithm. | Define typed expected frames from explicit values for the 13 existing cases. Keep both `future.infer_string` settings, `check_exact=True`, column/index checks, and the difference between replacing a dtype mapping and overriding individual entries. | +| Medium | [tests/pyathena/polars/test_result_set.py:259][polars-reader] | A generator is constructed in the parameter definition. Repeating the case can reuse a closed reader, making the post-close empty result a vacuous assertion. | Parameterize the reader kind and construct a fresh DataFrame or generator during each test invocation. Keep both case IDs. | +| Low | [tests/pyathena/pandas/test_result_set.py:79][filesystem-identities] | Module-level mocks are used only as filesystem identities while `read_parquet` is patched. They introduce mutable mock state without asserting any mock behavior. | Use distinct sentinels while preserving the identity and option-precedence assertions. | +| Low | [tests/pyathena/pandas/test_result_set.py:439][unused-dtypes] | The expected CSV parse contributes only a column-name comparison whose expected name is already literal. | Assert the explicit column name and preserved `007` value directly. | +| Medium | [AGENTS.md:84][grouping-rule], [tests/pyathena/test_parser.py:18][parser-group], and [tests/pyathena/pandas/test_util.py:62][utility-integration] | The grouping guidance does not explain established object-oriented unit classes or function-oriented utility tests that use AWS fixtures. Applying it mechanically changes test selection without improving the assertions. | Clarify the default forms and their purposeful exceptions in AGENTS.md and the testing guide. Retain the existing grouping and fixture scopes. | + +The all-types expectation shares a typed literal frame across cases. +Its time-only field still uses pandas' datetime conversion to supply today's date, as the existing input requires. +It does not parse a reference CSV or reproduce PyAthena's CSV conversion loop. + +## Intentional differences retained + +| Evidence | Rationale and disposition | +| --- | --- | +| [tests/pyathena/test_converter.py:19][converter-functions] and [tests/pyathena/test_parser.py:18][parser-group] | Standalone conversion functions and classes grouping a parser object's behavior are both useful. Preserve both forms. | +| [tests/pyathena/pandas/test_util.py:451][utility-writes] | Utility functions that write DataFrames use real cursor fixtures. Their function-oriented grouping does not make them offline tests or require a class conversion. | +| [tests/pyathena/pandas/test_result_set.py:52][whole-read] | Comparing joined chunks with a whole-file pandas read directly expresses the iterator's compatibility contract. Keep this library oracle. | +| [tests/pyathena/pandas/test_result_set.py:139][ddl-read] | The DDL test combines C-engine comparison with literal numeric-looking names, engine selection, and stream-closure assertions. These observable assertions keep the comparison tied to its specific contract. | +| [tests/pyathena/pandas/test_cursor.py:1945][pandas-converter] and [tests/pyathena/s3fs/test_cursor.py:543][s3fs-converter] | These parameters construct separate, configured converter values for explicit default/managed cases. Unlike a one-shot reader, they are not consumed. Keep the fixture inputs and managed-storage skip conditions; a future change that mutates the supplied converter should give it a per-invocation lifetime. | +| [tests/pyathena/sqlalchemy/test_compiler.py:621][compiler-types] | SQLAlchemy type and expression construction is part of the compiler input. Keep these declarative parameters and their dialect variants. Primitive parameter values also have useful generated IDs; custom IDs are needed when the generated names obscure the scenario. | +| [tests/sqlalchemy/test_suite.py:35][upstream-suite] and [tests/sqlalchemy/conftest.py:11][upstream-plugin] | Compliance tests inherit upstream classes and use SQLAlchemy's combinations, requirements, and pytest plugin. Preserve these interfaces and the sync/async database selection. | +| [tests/sqlalchemy/test_suite.py:974][explicit-skips] | Skipped overrides carry concrete Athena limitations. They differ from the unmarked filesystem placeholders because they report missing coverage as skipped. Preserve the reasons and test selection. | +| [NOTICE:29][adapted-tests] | The notice identifies surviving PyHive-derived cursor/dialect tests, including a native-async port. Preserve attribution and framework behavior when reorganizing adapted material. | +| [tests/pyathena/conftest.py:225][cursor-lifetime] and [tests/pyathena/aio/conftest.py:21][aio-lifetime] | Sync and native-async fixtures own their connections and cursor contexts with their corresponding teardown forms. The implementations need different syntax; a shared replacement is not required for consistency. | +| [tests/pyathena/arrow/test_cursor.py:38][arrow-binary] and [tests/pyathena/aio/arrow/test_cursor.py:22][aio-arrow-binary] | Both execution styles assert binary NULL versus empty bytes. Backend-specific cases and Future versus native-async result handling justify differences in the complete method inventories. Preserve meaningful corresponding scenarios rather than forcing identical test lists. | + +The session hooks in [tests/pyathena/conftest.py:15][session-hooks] prepare real AWS resources even for a selected pure-logic test. +This existing behavior is documented in the testing guide. +The organizational changes preserve it; an offline invocation must explicitly exclude those hooks and select self-contained modules. + +## Validation boundaries + +The initial audit is static source inspection. +Runtime results belong to the implementation revision and are recorded separately in the pull request's TEST section, with commands, dependency versions, and skipped coverage. +Compare the affected modules' collected node IDs before and after the changes, then run the self-contained pandas and Polars tests offline and both disk-usage cases against the configured AWS environment. +Normal CI subsequently exercises the applicable PyAthena suite. + +The fixes above address the identified actionable inconsistencies without a broad grouping conversion. +Additional test-correctness findings from implementation review should be evaluated against the same observable-contract and fixture-lifetime criteria. + +## File inventory + +```text +tests/__init__.py +tests/pyathena/__init__.py +tests/pyathena/aio/__init__.py +tests/pyathena/aio/arrow/__init__.py +tests/pyathena/aio/arrow/test_cursor.py +tests/pyathena/aio/conftest.py +tests/pyathena/aio/pandas/__init__.py +tests/pyathena/aio/pandas/test_cursor.py +tests/pyathena/aio/polars/__init__.py +tests/pyathena/aio/polars/test_cursor.py +tests/pyathena/aio/s3fs/__init__.py +tests/pyathena/aio/s3fs/test_cursor.py +tests/pyathena/aio/spark/__init__.py +tests/pyathena/aio/spark/test_cursor.py +tests/pyathena/aio/sqlalchemy/__init__.py +tests/pyathena/aio/sqlalchemy/test_base.py +tests/pyathena/aio/test_common.py +tests/pyathena/aio/test_connection.py +tests/pyathena/aio/test_cursor.py +tests/pyathena/aio/test_result_set.py +tests/pyathena/arrow/__init__.py +tests/pyathena/arrow/test_async_cursor.py +tests/pyathena/arrow/test_converter.py +tests/pyathena/arrow/test_cursor.py +tests/pyathena/arrow/test_result_set.py +tests/pyathena/arrow/test_util.py +tests/pyathena/conftest.py +tests/pyathena/filesystem/__init__.py +tests/pyathena/filesystem/test_init.py +tests/pyathena/filesystem/test_s3.py +tests/pyathena/filesystem/test_s3_async.py +tests/pyathena/filesystem/test_s3_core.py +tests/pyathena/filesystem/test_s3_errors.py +tests/pyathena/filesystem/test_s3_executor.py +tests/pyathena/filesystem/test_s3_object.py +tests/pyathena/filesystem/test_s3_path.py +tests/pyathena/pandas/__init__.py +tests/pyathena/pandas/test_async_cursor.py +tests/pyathena/pandas/test_converter.py +tests/pyathena/pandas/test_cursor.py +tests/pyathena/pandas/test_reader.py +tests/pyathena/pandas/test_result_set.py +tests/pyathena/pandas/test_util.py +tests/pyathena/polars/__init__.py +tests/pyathena/polars/test_async_cursor.py +tests/pyathena/polars/test_converter.py +tests/pyathena/polars/test_cursor.py +tests/pyathena/polars/test_result_set.py +tests/pyathena/s3fs/__init__.py +tests/pyathena/s3fs/test_async_cursor.py +tests/pyathena/s3fs/test_cursor.py +tests/pyathena/s3fs/test_reader.py +tests/pyathena/spark/__init__.py +tests/pyathena/spark/test_async_cursor.py +tests/pyathena/spark/test_common.py +tests/pyathena/spark/test_spark_cursor.py +tests/pyathena/sqlalchemy/__init__.py +tests/pyathena/sqlalchemy/test_array.py +tests/pyathena/sqlalchemy/test_base.py +tests/pyathena/sqlalchemy/test_compiler.py +tests/pyathena/sqlalchemy/test_map.py +tests/pyathena/sqlalchemy/test_struct.py +tests/pyathena/sqlalchemy/test_temporal.py +tests/pyathena/sqlalchemy/test_types.py +tests/pyathena/tables.py +tests/pyathena/test_async_cursor.py +tests/pyathena/test_connection.py +tests/pyathena/test_converter.py +tests/pyathena/test_cursor.py +tests/pyathena/test_formatter.py +tests/pyathena/test_glue.py +tests/pyathena/test_model.py +tests/pyathena/test_options.py +tests/pyathena/test_parser.py +tests/pyathena/test_result_set.py +tests/pyathena/test_util.py +tests/pyathena/util.py +tests/resources/queries/create_database.sql.jinja2 +tests/resources/queries/drop_database.sql.jinja2 +tests/sqlalchemy/__init__.py +tests/sqlalchemy/conftest.py +tests/sqlalchemy/profiles.txt +tests/sqlalchemy/test_suite.py +``` + +[sync-du]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/filesystem/test_s3.py#L4432-L4434 +[async-du]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/filesystem/test_s3_async.py#L1491-L1493 +[csv-helper]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L265-L300 +[csv-params]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L305-L389 +[csv-oracle]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L391-L423 +[polars-reader]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/polars/test_result_set.py#L259-L268 +[filesystem-identities]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L79-L80 +[unused-dtypes]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L439-L452 +[grouping-rule]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/AGENTS.md#L84-L85 +[parser-group]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/test_parser.py#L18-L36 +[utility-integration]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_util.py#L62-L70 +[converter-functions]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/test_converter.py#L19-L30 +[utility-writes]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_util.py#L451-L471 +[whole-read]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L52-L58 +[ddl-read]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L139-L181 +[pandas-converter]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_cursor.py#L1945-L1971 +[s3fs-converter]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/s3fs/test_cursor.py#L543-L566 +[compiler-types]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/sqlalchemy/test_compiler.py#L621-L658 +[upstream-suite]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/sqlalchemy/test_suite.py#L35-L44 +[upstream-plugin]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/sqlalchemy/conftest.py#L11-L39 +[explicit-skips]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/sqlalchemy/test_suite.py#L974-L981 +[adapted-tests]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/NOTICE#L29-L36 +[cursor-lifetime]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/conftest.py#L225-L234 +[aio-lifetime]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/aio/conftest.py#L21-L32 +[arrow-binary]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/arrow/test_cursor.py#L38-L53 +[aio-arrow-binary]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/aio/arrow/test_cursor.py#L22-L36 +[session-hooks]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/conftest.py#L15-L56 diff --git a/docs/testing.md b/docs/testing.md index 575d1676e..d530d8320 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -147,6 +147,32 @@ Include failed and skipped tests and explain any unrun coverage. Separate real AWS results from mock-based tests and static checks. Sanitize logs before sharing them. +## Organize tests + +Group cursor and engine integration tests in classes, and use standalone functions for stateless helpers. +A unit-test class can group the behavior of one object or common setup, as in `TestTypeSignatureParser`. +Function-oriented utility tests can remain standalone when they use AWS fixtures; `tests/pyathena/pandas/test_util.py` follows this pattern. +SQLAlchemy compliance tests retain the classes, decorators, and plugin setup required by the upstream suite. +Preserve attribution for adapted tests as documented in `NOTICE`. + +Parameter definitions should make inputs, options, and expected behavior visible. +Build mocks, configured result sets, and one-shot readers during fixture setup or test execution. +Pure values and framework type or expression objects can be constructed in parameters. +Choose explicit parameter IDs when generated IDs obscure the case, and keep existing IDs, marks, and fixture scopes when reorganizing tests. + +Use literal expected values for contract-specific behavior. +A library comparison is useful when matching that library is the contract, such as joining pandas chunks versus reading the whole file. +Express intentional differences directly rather than recreating the implementation in the expected-value builder. +Retain dtype and schema checks, option contexts, resource cleanup, and equivalent synchronous and asynchronous scenarios where applicable. + +The [test conventions audit](testing-audit.md) records the reviewed areas, source evidence, improvements, and intentional exceptions for issue #1079. + +```{toctree} +:hidden: + +testing-audit +``` + ## GitHub Actions The Test workflow runs for pull requests that change files other than `docs/` and Markdown. diff --git a/tests/pyathena/filesystem/test_s3.py b/tests/pyathena/filesystem/test_s3.py index 9bdd00fdd..5af29ef48 100644 --- a/tests/pyathena/filesystem/test_s3.py +++ b/tests/pyathena/filesystem/test_s3.py @@ -4819,9 +4819,25 @@ def test_find_withdirs(self, fs): result = fs.find(dir_, withdirs=False) assert len(result) == 4 # Only files - def test_du(self): - # TODO - pass + def test_du(self, fs): + """Disk usage reports file sizes, their total, and the requested depth.""" + directory = ( + f"s3://{ENV.s3_staging_bucket}/{ENV.s3_staging_key}{ENV.schema}/filesystem/test_du" + ) + first = f"{directory}/first" + second = f"{directory}/nested/second" + try: + fs.pipe_file(first, b"abc") + fs.pipe_file(second, b"12345") + assert fs.du(directory) == 8 + assert fs.du(directory, total=False) == { + fs._strip_protocol(first): 3, + fs._strip_protocol(second): 5, + } + assert fs.du(directory, maxdepth=1) == 3 + assert fs.du(first) == 3 + finally: + fs.rm(directory, recursive=True) def test_glob(self, fs): dir_ = f"s3://{ENV.s3_staging_bucket}/{ENV.s3_staging_key}{ENV.schema}/filesystem/test_glob" diff --git a/tests/pyathena/filesystem/test_s3_async.py b/tests/pyathena/filesystem/test_s3_async.py index 19f442c50..af42bfa30 100644 --- a/tests/pyathena/filesystem/test_s3_async.py +++ b/tests/pyathena/filesystem/test_s3_async.py @@ -1775,9 +1775,24 @@ async def test_find_withdirs(self, fs): result = await fs._find(dir_, withdirs=False) assert len(result) == 4 # Only files - def test_du(self): - # TODO - pass + @pytest.mark.asyncio + async def test_du(self, fs): + """Disk usage reports file sizes, their total, and the requested depth.""" + directory = f"s3://{ENV.s3_staging_bucket}/{ENV.s3_staging_key}{ENV.schema}/filesystem/test_async_du" + first = f"{directory}/first" + second = f"{directory}/nested/second" + try: + await fs._pipe_file(first, b"abc") + await fs._pipe_file(second, b"12345") + assert await fs._du(directory) == 8 + assert await fs._du(directory, total=False) == { + fs._strip_protocol(first): 3, + fs._strip_protocol(second): 5, + } + assert await fs._du(directory, maxdepth=1) == 3 + assert await fs._du(first) == 3 + finally: + await fs._rm(directory, recursive=True) @pytest.mark.asyncio async def test_glob(self, fs): diff --git a/tests/pyathena/pandas/test_result_set.py b/tests/pyathena/pandas/test_result_set.py index 0a8cb680e..179594958 100644 --- a/tests/pyathena/pandas/test_result_set.py +++ b/tests/pyathena/pandas/test_result_set.py @@ -7,7 +7,7 @@ import csv import io -from unittest.mock import MagicMock, PropertyMock, patch +from unittest.mock import MagicMock, PropertyMock, patch, sentinel import pandas as pd import pyarrow as pa @@ -76,8 +76,8 @@ def test_as_pandas_single_dataframe(self): assert df_iter.as_pandas() is df -_FS = MagicMock(name="pyathena_fs") -_USER_FS = MagicMock(name="user_fs") +_FS = sentinel.pyathena_fs +_USER_FS = sentinel.user_fs class TestAthenaPandasResultSet: @@ -253,28 +253,20 @@ def test_read_parquet_filesystem(self, execute_kwargs, path, filesystem_kwargs): } -def _is_string_dtype(value): - """Return whether a dtype mapping value is a string dtype, ignoring invalid values.""" - try: - dtype = pd.api.types.pandas_dtype(value) - except TypeError: - return False - return isinstance(dtype, pd.StringDtype) or dtype.kind == "U" - - -def _pyarrow_read_csv_kwargs(types, tab_separated=False, **kwargs): +def _pyarrow_read_csv_kwargs(types, tab_separated=False, dtype_overrides=None, **kwargs): """Build the pandas.read_csv() options with AthenaPandasResultSet._get_csv_read_options(). Args: types: The Athena types of the result columns, keyed by column name. tab_separated: Whether the result is a tab-separated ``.txt`` file. + dtype_overrides: Entries to add to or replace in the default dtype mapping. + A ``dtype`` in kwargs instead replaces the entire mapping. **kwargs: The pandas.read_csv() options given to ``execute()``. Returns: The options for the PyArrow engine. """ - with patch("pyathena.pandas.result_set.AthenaResultSet.__init__", return_value=None): - result_set = AthenaPandasResultSet.__new__(AthenaPandasResultSet) + result_set = AthenaPandasResultSet.__new__(AthenaPandasResultSet) result_set._converter = DefaultPandasTypeConverter() result_set._keep_default_na = False result_set._na_values = ("",) @@ -296,6 +288,8 @@ def _pyarrow_read_csv_kwargs(types, tab_separated=False, **kwargs): return_value=location, ), ): + if dtype_overrides is not None: + result_set._kwargs["dtype"] = {**result_set.dtypes, **dtype_overrides} assert result_set._reads_csv_with_pyarrow() return result_set._get_csv_read_options("pyarrow", None) @@ -435,126 +429,202 @@ def test_read_csv_without_json_c_converter_keeps_low_memory_default(types, engin assert len(df) == 30 +def _string_series(values, infer_string): + """Express the expected string dtype for the active pandas option.""" + return pd.Series(values, dtype="str" if infer_string else object) + + +def _types_frame(infer_string, parse_time=True): + """Build the typed literal expectation for the all-types CSV.""" + missing = float("nan") + return pd.DataFrame( + { + "ti": pd.Series([1, None], dtype="Int64"), + "si": pd.Series([2, None], dtype="Int64"), + "i": pd.Series([3, None], dtype="Int64"), + "bi": pd.Series([4, None], dtype="Int64"), + "r": pd.Series([1.5, missing], dtype="float64"), + "d": pd.Series([2.25, missing], dtype="float64"), + "c": _string_series(["ab ", missing], infer_string), + "v": _string_series(["plain", missing], infer_string), + "ml": _string_series(['multi\nline "q", x', missing], infer_string), + "arr": _string_series(["[1, 2]", missing], infer_string), + "m": _string_series(["{k=1}", missing], infer_string), + "rw": _string_series(["{a=1, b=x}", missing], infer_string), + "dt": pd.Series([pd.Timestamp("2024-02-29"), pd.NaT], dtype="datetime64[us]"), + "ts": pd.Series( + [pd.Timestamp("2024-02-29 23:59:58.123"), pd.NaT], dtype="datetime64[ns]" + ), + "ts6": pd.Series( + [pd.Timestamp("2024-02-29 23:59:58.123456"), pd.NaT], dtype="datetime64[ns]" + ), + # pandas supplies today's date when it parses a time without a date. + "tm": ( + pd.Series(pd.to_datetime(["12:34:56.789", None])) + if parse_time + else _string_series(["12:34:56.789", None], infer_string) + ), + "iv": _string_series(["2 00:00:00.000", None], infer_string), + "nul": pd.Series([missing, missing], dtype="float64"), + "u": _string_series(["589f6631-9c50-4f58-a121-e2608a04fc64", None], infer_string), + "empty": _string_series([missing, missing], infer_string), + "na": _string_series(["NA", missing], infer_string), + } + ) + + @pytest.mark.filterwarnings("ignore:Could not infer format") @pytest.mark.parametrize("infer_string", [True, False]) @pytest.mark.parametrize( - ("data", "read_csv_kwargs"), + ("data", "types", "read_options", "expected_frame"), [ - (_TYPES_CSV, _pyarrow_read_csv_kwargs(_TYPES)), - ( + pytest.param(_TYPES_CSV, _TYPES, {}, _types_frame, id="types"), + pytest.param( _TYPES_CSV, - _pyarrow_read_csv_kwargs( - _TYPES, - dtype={ - **_pyarrow_read_csv_kwargs(_TYPES)["dtype"], - "ti": "float32", - "v": "category", - "missing": "int64", - }, - ), + _TYPES, + {"dtype_overrides": {"ti": "float32", "v": "category", "missing": "int64"}}, + lambda infer: _types_frame(infer).astype({"ti": "float32", "v": "category"}), + id="dtype", + ), + pytest.param( + _TYPES_CSV, + _TYPES, + {"parse_dates": [12, "ts"]}, + lambda infer: _types_frame(infer, parse_time=False), + id="parse_dates", ), - (_TYPES_CSV, _pyarrow_read_csv_kwargs(_TYPES, parse_dates=[12, "ts"])), - ( + pytest.param( _TYPES_CSV, - _pyarrow_read_csv_kwargs( - _TYPES, dtype={**_pyarrow_read_csv_kwargs(_TYPES)["dtype"], "dt": "string"} + _TYPES, + {"dtype_overrides": {"dt": "string"}}, + lambda infer: _types_frame(infer).assign( + dt=pd.Series(["2024-02-29", None], dtype="string") ), + id="dtype_of_date_column", ), - ( + pytest.param( '"x","d"\n"1","2024-01-01"\n,\n', - _pyarrow_read_csv_kwargs({"x": "integer", "d": "date"}, dtype={"x": None}), + {"x": "integer", "d": "date"}, + {"dtype": {"x": None}}, + lambda infer: pd.DataFrame( + { + "x": [1.0, float("nan")], + "d": pd.Series([pd.Timestamp("2024-01-01"), pd.NaT], dtype="datetime64[us]"), + } + ), + id="dtype_none", ), - ( + pytest.param( '"x","x","d"\n"1","2","2024-01-01"\n,,\n', - _pyarrow_read_csv_kwargs({"x": "integer", "d": "date"}), + {"x": "integer", "d": "date"}, + {}, + lambda infer: pd.concat( + [ + pd.Series([1, None], name="x", dtype="Int64"), + pd.Series([2, None], name="x", dtype="Int64"), + pd.Series( + [pd.Timestamp("2024-01-01"), pd.NaT], name="d", dtype="datetime64[us]" + ), + ], + axis=1, + ), + id="duplicate_names", ), - ( + pytest.param( '"v","n"\n"1","1"\n,\n"nan","3"\n"007","4"\n"1e3","5"\n', - _pyarrow_read_csv_kwargs({"v": "varchar", "n": "integer"}), + {"v": "varchar", "n": "integer"}, + {}, + lambda infer: pd.DataFrame( + { + "v": _string_series(["1", float("nan"), "nan", "007", "1e3"], infer), + "n": pd.Series([1, None, 3, 4, 5], dtype="Int64"), + } + ), + id="numeric_looking_strings", ), - ( + pytest.param( '"v","w","x"\n"007","a","1"\n,,"2"\n', - _pyarrow_read_csv_kwargs( - {"x": "integer"}, - dtype={ + {"x": "integer"}, + { + "dtype": { "v": pd.ArrowDtype(pa.string()), "w": pd.ArrowDtype(pa.large_string()), "x": pd.Int64Dtype(), - }, + } + }, + lambda infer: pd.DataFrame( + { + "v": pd.Series(["007", None], dtype=pd.ArrowDtype(pa.string())), + "w": pd.Series(["a", None], dtype=pd.ArrowDtype(pa.large_string())), + "x": pd.Series([1, 2], dtype="Int64"), + } ), + id="arrow_string_dtypes", ), - ( + pytest.param( "001\t2\t003\n004\t5\t\n", - _pyarrow_read_csv_kwargs({"v": "varchar"}, True), + {"v": "varchar"}, + {"tab_separated": True}, + lambda infer: pd.DataFrame( + {"0": [1, 4], "1": [2, 5], "v": _string_series(["3", float("nan")], infer)} + ), + id="tab_separated_numeric_fields", ), - ( + pytest.param( '"v","n"\n"007","1"\n', - _pyarrow_read_csv_kwargs( - {"v": "varchar", "n": "integer"}, - dtype={**_pyarrow_read_csv_kwargs({"v": "varchar"})["dtype"], 0: str}, - ), + {"v": "varchar", "n": "integer"}, + {"dtype": {"v": str, 0: str}}, + lambda infer: pd.DataFrame({"v": _string_series(["007"], infer), "n": [1]}), + id="dtype_position_key", ), - ( + pytest.param( '"v"\n"plain"\n"2024-01-01"\n\n', - _pyarrow_read_csv_kwargs({"v": "varchar"}, parse_dates=["v"]), + {"v": "varchar"}, + {"parse_dates": ["v"]}, + lambda infer: pd.DataFrame( + {"v": _string_series(["plain", "2024-01-01", float("nan")], infer)} + ), + id="unparsed_dates", ), - ( + pytest.param( "id \tint \t \nname \tstring \t \n", - _pyarrow_read_csv_kwargs({"col_name": "varchar"}, True), + {"col_name": "varchar"}, + {"tab_separated": True}, + lambda infer: pd.DataFrame( + { + "0": _string_series(["id ", "name "], infer), + "1": _string_series(["int ", "string "], infer), + "col_name": _string_series([" ", " "], infer), + } + ), + id="tab_separated_extra_fields", ), - ( + pytest.param( "x\t1\t2024-01-01\n\t\t\ny y\t3\t2024-01-02\n", - _pyarrow_read_csv_kwargs({"a": "varchar", "b": "bigint", "c": "date"}, True), + {"a": "varchar", "b": "bigint", "c": "date"}, + {"tab_separated": True}, + lambda infer: pd.DataFrame( + { + "a": _string_series(["x", float("nan"), "y y"], infer), + "b": pd.Series([1, None, 3], dtype="Int64"), + "c": pd.Series( + [pd.Timestamp("2024-01-01"), pd.NaT, pd.Timestamp("2024-01-02")], + dtype="datetime64[us]", + ), + } + ), + id="tab_separated", ), ], - ids=[ - "types", - "dtype", - "parse_dates", - "dtype_of_date_column", - "dtype_none", - "duplicate_names", - "numeric_looking_strings", - "arrow_string_dtypes", - "tab_separated_numeric_fields", - "dtype_position_key", - "unparsed_dates", - "tab_separated_extra_fields", - "tab_separated", - ], ) -def test_read_csv_with_pyarrow_matches_pandas(data, read_csv_kwargs, infer_string): - # Without values that cross a read block, the result is the one of - # pandas.read_csv(engine="pyarrow"), except that the columns with a string - # dtype have the values of pandas' C engine, and in a header-less file, its - # missing values. Where the C engine parses a parse_dates column despite its - # string dtype, the PyArrow engine's applying the dtype again is kept. +def test_read_csv_with_pyarrow_matches_pandas( + data, types, read_options, expected_frame, infer_string +): + """CSV values, dtypes, column names, and index match the case's pandas expectation.""" with pd.option_context("future.infer_string", infer_string): - expected = pd.read_csv( - io.BytesIO(data.encode()), - **{**read_csv_kwargs, "dtype": dict(read_csv_kwargs["dtype"])}, - ) - c_engine = pd.read_csv( - io.BytesIO(data.encode()), - **{**read_csv_kwargs, "engine": "c", "dtype": dict(read_csv_kwargs["dtype"])}, - ) - string_columns = { - column for column, value in read_csv_kwargs["dtype"].items() if _is_string_dtype(value) - } - for index, column in enumerate(expected.columns): - if column not in string_columns or c_engine[column].dtype.kind == "M": - continue - if read_csv_kwargs["header"] is None: - # Header-less fields keep the inferred types, and only their missing - # values follow the C engine, which makes extra fields the index. - missing = c_engine[column].isna().to_numpy() - expected.isetitem(index, expected.iloc[:, index].mask(missing, float("nan"))) - else: - expected.isetitem(index, c_engine[column].array) - actual = _read_csv_with_pyarrow( - io.BytesIO(data.encode()), - {**read_csv_kwargs, "dtype": dict(read_csv_kwargs["dtype"])}, - ) + read_csv_kwargs = _pyarrow_read_csv_kwargs(types, **read_options) + expected = expected_frame(infer_string) + actual = _read_csv_with_pyarrow(io.BytesIO(data.encode()), read_csv_kwargs) assert_frame_equal(actual, expected, check_exact=True) @@ -579,11 +649,10 @@ def test_read_csv_with_pyarrow_ignores_unused_dtype_entries(): dtype={"v": str, "unused": "not-a-dtype", "unsupported": "decimal128(10, 2)[pyarrow]"}, ) data = b'"v"\n"007"\n' - expected = pd.read_csv(io.BytesIO(data), **{**read_csv_kwargs, "dtype": {"v": str}}) actual = _read_csv_with_pyarrow( io.BytesIO(data), {**read_csv_kwargs, "dtype": dict(read_csv_kwargs["dtype"])} ) - assert actual.columns.tolist() == expected.columns.tolist() == ["v"] + assert actual.columns.tolist() == ["v"] assert actual["v"].tolist() == ["007"] diff --git a/tests/pyathena/polars/test_result_set.py b/tests/pyathena/polars/test_result_set.py index a39fbcb1a..7cec99ea4 100644 --- a/tests/pyathena/polars/test_result_set.py +++ b/tests/pyathena/polars/test_result_set.py @@ -258,11 +258,16 @@ def test_read_csv_truncates_timestamps(self, kwargs, expected): class TestPolarsDataFrameIterator: @pytest.mark.parametrize( "reader", - [pl.DataFrame({"a": [1, 2]}), (df for df in [pl.DataFrame({"a": [1]})] * 2)], + ["dataframe", "generator"], ids=["dataframe", "generator"], ) def test_close_stops_iteration(self, reader): """A closed iterator yields nothing for either reader kind.""" + reader = ( + pl.DataFrame({"a": [1, 2]}) + if reader == "dataframe" + else (pl.DataFrame({"a": [1]}) for _ in range(2)) + ) df_iter = PolarsDataFrameIterator(reader, {}, ["a"]) df_iter.close() assert list(df_iter) == [] From 93a6e7fb8c6e99275480e857516adb0421e147df Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sun, 4 Oct 2026 17:57:11 +0900 Subject: [PATCH 2/6] Preserve pandas parity and clarify audit validation after review --- docs/testing-audit.md | 21 +++++++----- docs/testing.md | 26 +++++++++++++++ tests/pyathena/filesystem/test_s3.py | 3 +- tests/pyathena/filesystem/test_s3_async.py | 3 +- tests/pyathena/pandas/test_result_set.py | 39 +++++++++++++++++++--- 5 files changed, 76 insertions(+), 16 deletions(-) diff --git a/docs/testing-audit.md b/docs/testing-audit.md index b5669ef9d..3632fd54f 100644 --- a/docs/testing-audit.md +++ b/docs/testing-audit.md @@ -9,10 +9,10 @@ SPDX-License-Identifier: MIT # Test conventions audit -This report records the source audit for [issue #1079](https://github.com/pyathena-dev/PyAthena/issues/1079). +This report records the source audit conducted on October 4, 2026, for [issue #1079](https://github.com/pyathena-dev/PyAthena/issues/1079). The baseline is commit `200762088e7b0bac45054f22e32a16aa0ce95dbb`. Source references below identify that baseline, so later edits do not change their meaning. -The accompanying changes clarify the conventions and address the findings listed here. +The findings and dispositions describe that audit and its implementation in [PR #1082](https://github.com/pyathena-dev/PyAthena/pull/1082). ## Reviewed areas @@ -40,7 +40,7 @@ The complete file inventory appears below. The compliance suite imports additional tests from the installed SQLAlchemy package. Those upstream implementations and generated cases are outside the project-owned source inventory. The audit does not establish complete runtime coverage or the correctness of every assertion. -Unmerged worktrees, including the pandas JSON changes associated with issue #1078, are outside this baseline. +The audit excluded changes not merged into this baseline, including the pandas JSON changes associated with issue #1078. ## Findings addressed @@ -50,7 +50,7 @@ The priorities describe coverage and maintenance impact, rather than product-def | --- | --- | --- | --- | | High | [tests/pyathena/filesystem/test_s3.py:4432][sync-du] and [tests/pyathena/filesystem/test_s3_async.py:1491][async-du] | Both `test_du` bodies contain only `pass`. A broken disk-usage implementation still produces two passing tests. | Replace both placeholders with real S3 file-size, total, depth, and single-file assertions. Preserve the names, use the existing filesystem fixtures, and remove the test objects in `finally`. | | Medium | [tests/pyathena/pandas/test_result_set.py:265][csv-helper] and [tests/pyathena/pandas/test_result_set.py:305][csv-params] | Parameter evaluation repeatedly patches result-set properties and derives CSV options, including repeated default dtype construction. An option-building error prevents collection of unrelated tests in the module. | Store input metadata and options in the parameters. Build the options once per invocation, inside the pandas option context. Remove the unused base-initializer patch; `__new__` already bypasses `__init__`. | -| Medium | [tests/pyathena/pandas/test_result_set.py:391][csv-oracle] | The expectation reads the CSV with two engines and conditionally replaces columns and missing values. The reader has to reconstruct the intended contract from this algorithm. | Define typed expected frames from explicit values for the 13 existing cases. Keep both `future.infer_string` settings, `check_exact=True`, column/index checks, and the difference between replacing a dtype mapping and overriding individual entries. | +| Medium | [tests/pyathena/pandas/test_result_set.py:391][csv-oracle] | The expectation reads the CSV with two engines and conditionally replaces columns and missing values. The reader has to reconstruct the intended contract from this algorithm. | Define typed expected frames from explicit values for the 13 existing cases. Retain direct pandas PyArrow-engine comparisons for explicitly selected column positions where the contracts agree. Keep both `future.infer_string` settings, `check_exact=True`, column/index checks, and the difference between replacing a dtype mapping and overriding individual entries. | | Medium | [tests/pyathena/polars/test_result_set.py:259][polars-reader] | A generator is constructed in the parameter definition. Repeating the case can reuse a closed reader, making the post-close empty result a vacuous assertion. | Parameterize the reader kind and construct a fresh DataFrame or generator during each test invocation. Keep both case IDs. | | Low | [tests/pyathena/pandas/test_result_set.py:79][filesystem-identities] | Module-level mocks are used only as filesystem identities while `read_parquet` is patched. They introduce mutable mock state without asserting any mock behavior. | Use distinct sentinels while preserving the identity and option-precedence assertions. | | Low | [tests/pyathena/pandas/test_result_set.py:439][unused-dtypes] | The expected CSV parse contributes only a column-name comparison whose expected name is already literal. | Assert the explicit column name and preserved `007` value directly. | @@ -58,7 +58,9 @@ The priorities describe coverage and maintenance impact, rather than product-def The all-types expectation shares a typed literal frame across cases. Its time-only field still uses pandas' datetime conversion to supply today's date, as the existing input requires. -It does not parse a reference CSV or reproduce PyAthena's CSV conversion loop. +The literal frame does not parse a reference CSV or reproduce PyAthena's CSV conversion loop. +Separate library comparisons cover numeric, temporal, unmapped, and explicitly categorized columns, including duplicate and headerless column names. +Mapped string columns retain literal assertions for PyAthena's intentional preservation of string values and missing values. ## Intentional differences retained @@ -79,16 +81,17 @@ It does not parse a reference CSV or reproduce PyAthena's CSV conversion loop. The session hooks in [tests/pyathena/conftest.py:15][session-hooks] prepare real AWS resources even for a selected pure-logic test. This existing behavior is documented in the testing guide. The organizational changes preserve it; an offline invocation must explicitly exclude those hooks and select self-contained modules. +The {ref}`offline test instructions ` include the required placeholder environment values and a validated command. ## Validation boundaries The initial audit is static source inspection. -Runtime results belong to the implementation revision and are recorded separately in the pull request's TEST section, with commands, dependency versions, and skipped coverage. -Compare the affected modules' collected node IDs before and after the changes, then run the self-contained pandas and Polars tests offline and both disk-usage cases against the configured AWS environment. -Normal CI subsequently exercises the applicable PyAthena suite. +Runtime results belong to the implementation revision and are recorded separately in [PR #1082's TEST section](https://github.com/pyathena-dev/PyAthena/pull/1082), with commands, dependency versions, and skipped coverage. +The validation scope comprises comparison of the affected modules' collected node IDs, the self-contained pandas and Polars tests offline, and both disk-usage cases against the configured AWS environment. +The applicable PyAthena suite runs in CI when the implementation PR is Ready. The fixes above address the identified actionable inconsistencies without a broad grouping conversion. -Additional test-correctness findings from implementation review should be evaluated against the same observable-contract and fixture-lifetime criteria. +Implementation review also retained the pandas compatibility comparisons, documented the offline invocation, and protected the disk-usage tests' original write errors when cleanup encounters an absent prefix. ## File inventory diff --git a/docs/testing.md b/docs/testing.md index d530d8320..701667d6f 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -128,6 +128,32 @@ uv run --env-file .env pytest -n 1 tests/pyathena/test_cursor.py -v ``` A targeted run helps during development but does not replace other coverage required by the affected callers or features. + +(testing-offline)= + +### Run self-contained tests offline + +The pandas and Polars result-set modules have self-contained tests that can run without AWS access when the session hooks are excluded. +After `just lint`, run: + +```bash +env AWS_DEFAULT_REGION=us-east-1 \ + AWS_ATHENA_S3_STAGING_DIR=s3://pyathena-offline-placeholder/ \ + AWS_ATHENA_WORKGROUP=offline \ + AWS_ATHENA_SPARK_WORKGROUP=offline \ + AWS_EC2_METADATA_DISABLED=true \ + uv run pytest --noconftest -p no:rerunfailures -q \ + tests/pyathena/pandas/test_result_set.py \ + tests/pyathena/polars/test_result_set.py +``` + +The four placeholder AWS configuration values satisfy `tests/__init__.py`, which pytest still imports with `--noconftest`. +Disabling EC2 metadata prevents implicit credential lookup through that service. +`--noconftest` excludes the AWS session hooks and fixtures; disabling the rerun plugin also avoids its local socket setup in restricted environments. +Use this invocation only for self-contained modules; integration tests need their normal fixtures and a real AWS environment. + +### SQLAlchemy suites + The SQLAlchemy compliance suites under `tests/sqlalchemy/` run with different configurations: use `sqla` for synchronous dialects and `sqla-async` for native asyncio dialects. They do not run PyAthena's own dialect regression tests under `tests/pyathena/sqlalchemy/` and `tests/pyathena/aio/sqlalchemy/`. Run the relevant PyAthena tests too, either through `just test pyathena` or a focused selection during development: diff --git a/tests/pyathena/filesystem/test_s3.py b/tests/pyathena/filesystem/test_s3.py index 5af29ef48..7565fb5cb 100644 --- a/tests/pyathena/filesystem/test_s3.py +++ b/tests/pyathena/filesystem/test_s3.py @@ -4837,7 +4837,8 @@ def test_du(self, fs): assert fs.du(directory, maxdepth=1) == 3 assert fs.du(first) == 3 finally: - fs.rm(directory, recursive=True) + with contextlib.suppress(FileNotFoundError): + fs.rm(directory, recursive=True) def test_glob(self, fs): dir_ = f"s3://{ENV.s3_staging_bucket}/{ENV.s3_staging_key}{ENV.schema}/filesystem/test_glob" diff --git a/tests/pyathena/filesystem/test_s3_async.py b/tests/pyathena/filesystem/test_s3_async.py index af42bfa30..d77b3fff2 100644 --- a/tests/pyathena/filesystem/test_s3_async.py +++ b/tests/pyathena/filesystem/test_s3_async.py @@ -1792,7 +1792,8 @@ async def test_du(self, fs): assert await fs._du(directory, maxdepth=1) == 3 assert await fs._du(first) == 3 finally: - await fs._rm(directory, recursive=True) + with contextlib.suppress(FileNotFoundError): + await fs._rm(directory, recursive=True) @pytest.mark.asyncio async def test_glob(self, fs): diff --git a/tests/pyathena/pandas/test_result_set.py b/tests/pyathena/pandas/test_result_set.py index 179594958..93b2c9c3d 100644 --- a/tests/pyathena/pandas/test_result_set.py +++ b/tests/pyathena/pandas/test_result_set.py @@ -476,14 +476,22 @@ def _types_frame(infer_string, parse_time=True): @pytest.mark.filterwarnings("ignore:Could not infer format") @pytest.mark.parametrize("infer_string", [True, False]) @pytest.mark.parametrize( - ("data", "types", "read_options", "expected_frame"), + ("data", "types", "read_options", "expected_frame", "pandas_columns"), [ - pytest.param(_TYPES_CSV, _TYPES, {}, _types_frame, id="types"), + pytest.param( + _TYPES_CSV, + _TYPES, + {}, + _types_frame, + [0, 1, 2, 3, 4, 5, 12, 13, 14, 15, 16, 17, 18], + id="types", + ), pytest.param( _TYPES_CSV, _TYPES, {"dtype_overrides": {"ti": "float32", "v": "category", "missing": "int64"}}, lambda infer: _types_frame(infer).astype({"ti": "float32", "v": "category"}), + [0, 1, 2, 3, 4, 5, 7, 12, 13, 14, 15, 16, 17, 18], id="dtype", ), pytest.param( @@ -491,6 +499,7 @@ def _types_frame(infer_string, parse_time=True): _TYPES, {"parse_dates": [12, "ts"]}, lambda infer: _types_frame(infer, parse_time=False), + [0, 1, 2, 3, 4, 5, 12, 13, 14, 15, 16, 17, 18], id="parse_dates", ), pytest.param( @@ -500,6 +509,7 @@ def _types_frame(infer_string, parse_time=True): lambda infer: _types_frame(infer).assign( dt=pd.Series(["2024-02-29", None], dtype="string") ), + [0, 1, 2, 3, 4, 5, 12, 13, 14, 15, 16, 17, 18], id="dtype_of_date_column", ), pytest.param( @@ -512,6 +522,7 @@ def _types_frame(infer_string, parse_time=True): "d": pd.Series([pd.Timestamp("2024-01-01"), pd.NaT], dtype="datetime64[us]"), } ), + [0, 1], id="dtype_none", ), pytest.param( @@ -528,6 +539,7 @@ def _types_frame(infer_string, parse_time=True): ], axis=1, ), + [0, 1, 2], id="duplicate_names", ), pytest.param( @@ -540,6 +552,7 @@ def _types_frame(infer_string, parse_time=True): "n": pd.Series([1, None, 3, 4, 5], dtype="Int64"), } ), + [1], id="numeric_looking_strings", ), pytest.param( @@ -559,6 +572,7 @@ def _types_frame(infer_string, parse_time=True): "x": pd.Series([1, 2], dtype="Int64"), } ), + [2], id="arrow_string_dtypes", ), pytest.param( @@ -568,6 +582,7 @@ def _types_frame(infer_string, parse_time=True): lambda infer: pd.DataFrame( {"0": [1, 4], "1": [2, 5], "v": _string_series(["3", float("nan")], infer)} ), + [0, 1], id="tab_separated_numeric_fields", ), pytest.param( @@ -575,6 +590,7 @@ def _types_frame(infer_string, parse_time=True): {"v": "varchar", "n": "integer"}, {"dtype": {"v": str, 0: str}}, lambda infer: pd.DataFrame({"v": _string_series(["007"], infer), "n": [1]}), + [1], id="dtype_position_key", ), pytest.param( @@ -584,6 +600,7 @@ def _types_frame(infer_string, parse_time=True): lambda infer: pd.DataFrame( {"v": _string_series(["plain", "2024-01-01", float("nan")], infer)} ), + [], id="unparsed_dates", ), pytest.param( @@ -597,6 +614,7 @@ def _types_frame(infer_string, parse_time=True): "col_name": _string_series([" ", " "], infer), } ), + [0, 1, 2], id="tab_separated_extra_fields", ), pytest.param( @@ -613,19 +631,30 @@ def _types_frame(infer_string, parse_time=True): ), } ), + [1, 2], id="tab_separated", ), ], ) def test_read_csv_with_pyarrow_matches_pandas( - data, types, read_options, expected_frame, infer_string + data, types, read_options, expected_frame, pandas_columns, infer_string ): - """CSV values, dtypes, column names, and index match the case's pandas expectation.""" + """CSV results match literal expectations and pandas where their contracts agree.""" with pd.option_context("future.infer_string", infer_string): read_csv_kwargs = _pyarrow_read_csv_kwargs(types, **read_options) expected = expected_frame(infer_string) actual = _read_csv_with_pyarrow(io.BytesIO(data.encode()), read_csv_kwargs) - assert_frame_equal(actual, expected, check_exact=True) + assert_frame_equal(actual, expected, check_exact=True) + # Explicit positions retain duplicate names and headerless-column parity. + # Mapped string columns use PyAthena's preservation contract instead. + if pandas_columns: + reference = pd.read_csv( + io.BytesIO(data.encode()), + **{**read_csv_kwargs, "dtype": dict(read_csv_kwargs["dtype"])}, + ) + assert_frame_equal( + actual.iloc[:, pandas_columns], reference.iloc[:, pandas_columns], check_exact=True + ) @pytest.mark.parametrize("infer_string", [True, False]) From 60be248a0e4fced0e8fbe2d30dfe00c228582689 Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sun, 4 Oct 2026 18:08:23 +0900 Subject: [PATCH 3/6] Keep general test validation guidance outside SQLAlchemy section --- docs/testing.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/testing.md b/docs/testing.md index 701667d6f..1b99a99d7 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -162,12 +162,16 @@ Run the relevant PyAthena tests too, either through `just test pyathena` or a fo uv run --env-file .env pytest -n 1 tests/pyathena/sqlalchemy/ tests/pyathena/aio/sqlalchemy/ -v ``` +### Run tox + To invoke the configured tox environments locally: ```bash uv run --env-file .env just tox ``` +### Record results + Record the tested commit, Python and relevant dependency versions, exact commands, and results in the pull request. Include failed and skipped tests and explain any unrun coverage. Separate real AWS results from mock-based tests and static checks. From 03beabd2b4c6feb6686bbced597ccfb097d6cd68 Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sun, 4 Oct 2026 21:44:31 +0900 Subject: [PATCH 4/6] Keep the historical test audit in the PR discussion --- docs/testing-audit.md | 210 ------------------------------------------ docs/testing.md | 8 -- 2 files changed, 218 deletions(-) delete mode 100644 docs/testing-audit.md diff --git a/docs/testing-audit.md b/docs/testing-audit.md deleted file mode 100644 index 3632fd54f..000000000 --- a/docs/testing-audit.md +++ /dev/null @@ -1,210 +0,0 @@ - - -# Test conventions audit - -This report records the source audit conducted on October 4, 2026, for [issue #1079](https://github.com/pyathena-dev/PyAthena/issues/1079). -The baseline is commit `200762088e7b0bac45054f22e32a16aa0ce95dbb`. -Source references below identify that baseline, so later edits do not change their meaning. -The findings and dispositions describe that audit and its implementation in [PR #1082](https://github.com/pyathena-dev/PyAthena/pull/1082). - -## Reviewed areas - -The inventory covers all 83 tracked files under `tests/`: 80 Python files, two Jinja query templates, and the SQLAlchemy profiles file. -The Python inventory includes 17 package initializers and all three `conftest.py` files. -Structural inspection covered class and function organization, fixture arguments and decorators, parameter construction, helper placement, assertions, and synchronous and asynchronous counterparts. -Focused source tracing checked the setup and expected-value patterns identified by that inspection. -The complete file inventory appears below. - -| Area | Files | Conventions examined | -| --- | ---: | --- | -| Shared PyAthena tests and helpers | 15 | Cursor integration classes; standalone conversion helpers; object-oriented parser, model, result-set, and connection tests; session data and resource setup | -| Native asyncio tests and fixtures | 18 | Async cursor and dialect classes, async result-set helpers, per-test cursor contexts, connection teardown, and backend-specific cases | -| Arrow | 6 | Cursor classes, converter functions and classes, schema and precision assertions, mocked result-set setup | -| pandas | 7 | Cursor classes, function-oriented utility tests, reader lifetimes, CSV options and expected frames, dtype and option-context coverage | -| Polars | 5 | Cursor and converter tests, typed local files, failure after partial reads, and one-shot iterator inputs | -| S3FS cursors and readers | 4 | Cursor fixtures, custom converters, CSV NULL versus empty-string assertions, and reader context managers | -| Spark | 4 | Session and calculation fixtures, event-based concurrency helpers, cancellation assertions, and Future-based async behavior | -| Filesystem | 9 | Class fixtures, stubbed provider calls, path/value parametrization, sync/async operations, cleanup, and tests without observable assertions | -| PyAthena SQLAlchemy tests | 8 | Compiler and type unit tests, engine integration classes, local fixture reuse, SQL expression parameters, and reflected metadata assertions | -| SQLAlchemy compliance suite | 4 | Upstream inheritance, combinations and requirements, plugin hooks, sync/async selection, and explicit skips | -| Shared environment initializer | 1 | Required environment values and per-process schema identity | -| Query templates | 2 | Database setup/teardown, schema interpolation, and license notices | - -The compliance suite imports additional tests from the installed SQLAlchemy package. -Those upstream implementations and generated cases are outside the project-owned source inventory. -The audit does not establish complete runtime coverage or the correctness of every assertion. -The audit excluded changes not merged into this baseline, including the pandas JSON changes associated with issue #1078. - -## Findings addressed - -The priorities describe coverage and maintenance impact, rather than product-defect severity. - -| Priority | Baseline evidence | Impact | Disposition | -| --- | --- | --- | --- | -| High | [tests/pyathena/filesystem/test_s3.py:4432][sync-du] and [tests/pyathena/filesystem/test_s3_async.py:1491][async-du] | Both `test_du` bodies contain only `pass`. A broken disk-usage implementation still produces two passing tests. | Replace both placeholders with real S3 file-size, total, depth, and single-file assertions. Preserve the names, use the existing filesystem fixtures, and remove the test objects in `finally`. | -| Medium | [tests/pyathena/pandas/test_result_set.py:265][csv-helper] and [tests/pyathena/pandas/test_result_set.py:305][csv-params] | Parameter evaluation repeatedly patches result-set properties and derives CSV options, including repeated default dtype construction. An option-building error prevents collection of unrelated tests in the module. | Store input metadata and options in the parameters. Build the options once per invocation, inside the pandas option context. Remove the unused base-initializer patch; `__new__` already bypasses `__init__`. | -| Medium | [tests/pyathena/pandas/test_result_set.py:391][csv-oracle] | The expectation reads the CSV with two engines and conditionally replaces columns and missing values. The reader has to reconstruct the intended contract from this algorithm. | Define typed expected frames from explicit values for the 13 existing cases. Retain direct pandas PyArrow-engine comparisons for explicitly selected column positions where the contracts agree. Keep both `future.infer_string` settings, `check_exact=True`, column/index checks, and the difference between replacing a dtype mapping and overriding individual entries. | -| Medium | [tests/pyathena/polars/test_result_set.py:259][polars-reader] | A generator is constructed in the parameter definition. Repeating the case can reuse a closed reader, making the post-close empty result a vacuous assertion. | Parameterize the reader kind and construct a fresh DataFrame or generator during each test invocation. Keep both case IDs. | -| Low | [tests/pyathena/pandas/test_result_set.py:79][filesystem-identities] | Module-level mocks are used only as filesystem identities while `read_parquet` is patched. They introduce mutable mock state without asserting any mock behavior. | Use distinct sentinels while preserving the identity and option-precedence assertions. | -| Low | [tests/pyathena/pandas/test_result_set.py:439][unused-dtypes] | The expected CSV parse contributes only a column-name comparison whose expected name is already literal. | Assert the explicit column name and preserved `007` value directly. | -| Medium | [AGENTS.md:84][grouping-rule], [tests/pyathena/test_parser.py:18][parser-group], and [tests/pyathena/pandas/test_util.py:62][utility-integration] | The grouping guidance does not explain established object-oriented unit classes or function-oriented utility tests that use AWS fixtures. Applying it mechanically changes test selection without improving the assertions. | Clarify the default forms and their purposeful exceptions in AGENTS.md and the testing guide. Retain the existing grouping and fixture scopes. | - -The all-types expectation shares a typed literal frame across cases. -Its time-only field still uses pandas' datetime conversion to supply today's date, as the existing input requires. -The literal frame does not parse a reference CSV or reproduce PyAthena's CSV conversion loop. -Separate library comparisons cover numeric, temporal, unmapped, and explicitly categorized columns, including duplicate and headerless column names. -Mapped string columns retain literal assertions for PyAthena's intentional preservation of string values and missing values. - -## Intentional differences retained - -| Evidence | Rationale and disposition | -| --- | --- | -| [tests/pyathena/test_converter.py:19][converter-functions] and [tests/pyathena/test_parser.py:18][parser-group] | Standalone conversion functions and classes grouping a parser object's behavior are both useful. Preserve both forms. | -| [tests/pyathena/pandas/test_util.py:451][utility-writes] | Utility functions that write DataFrames use real cursor fixtures. Their function-oriented grouping does not make them offline tests or require a class conversion. | -| [tests/pyathena/pandas/test_result_set.py:52][whole-read] | Comparing joined chunks with a whole-file pandas read directly expresses the iterator's compatibility contract. Keep this library oracle. | -| [tests/pyathena/pandas/test_result_set.py:139][ddl-read] | The DDL test combines C-engine comparison with literal numeric-looking names, engine selection, and stream-closure assertions. These observable assertions keep the comparison tied to its specific contract. | -| [tests/pyathena/pandas/test_cursor.py:1945][pandas-converter] and [tests/pyathena/s3fs/test_cursor.py:543][s3fs-converter] | These parameters construct separate, configured converter values for explicit default/managed cases. Unlike a one-shot reader, they are not consumed. Keep the fixture inputs and managed-storage skip conditions; a future change that mutates the supplied converter should give it a per-invocation lifetime. | -| [tests/pyathena/sqlalchemy/test_compiler.py:621][compiler-types] | SQLAlchemy type and expression construction is part of the compiler input. Keep these declarative parameters and their dialect variants. Primitive parameter values also have useful generated IDs; custom IDs are needed when the generated names obscure the scenario. | -| [tests/sqlalchemy/test_suite.py:35][upstream-suite] and [tests/sqlalchemy/conftest.py:11][upstream-plugin] | Compliance tests inherit upstream classes and use SQLAlchemy's combinations, requirements, and pytest plugin. Preserve these interfaces and the sync/async database selection. | -| [tests/sqlalchemy/test_suite.py:974][explicit-skips] | Skipped overrides carry concrete Athena limitations. They differ from the unmarked filesystem placeholders because they report missing coverage as skipped. Preserve the reasons and test selection. | -| [NOTICE:29][adapted-tests] | The notice identifies surviving PyHive-derived cursor/dialect tests, including a native-async port. Preserve attribution and framework behavior when reorganizing adapted material. | -| [tests/pyathena/conftest.py:225][cursor-lifetime] and [tests/pyathena/aio/conftest.py:21][aio-lifetime] | Sync and native-async fixtures own their connections and cursor contexts with their corresponding teardown forms. The implementations need different syntax; a shared replacement is not required for consistency. | -| [tests/pyathena/arrow/test_cursor.py:38][arrow-binary] and [tests/pyathena/aio/arrow/test_cursor.py:22][aio-arrow-binary] | Both execution styles assert binary NULL versus empty bytes. Backend-specific cases and Future versus native-async result handling justify differences in the complete method inventories. Preserve meaningful corresponding scenarios rather than forcing identical test lists. | - -The session hooks in [tests/pyathena/conftest.py:15][session-hooks] prepare real AWS resources even for a selected pure-logic test. -This existing behavior is documented in the testing guide. -The organizational changes preserve it; an offline invocation must explicitly exclude those hooks and select self-contained modules. -The {ref}`offline test instructions ` include the required placeholder environment values and a validated command. - -## Validation boundaries - -The initial audit is static source inspection. -Runtime results belong to the implementation revision and are recorded separately in [PR #1082's TEST section](https://github.com/pyathena-dev/PyAthena/pull/1082), with commands, dependency versions, and skipped coverage. -The validation scope comprises comparison of the affected modules' collected node IDs, the self-contained pandas and Polars tests offline, and both disk-usage cases against the configured AWS environment. -The applicable PyAthena suite runs in CI when the implementation PR is Ready. - -The fixes above address the identified actionable inconsistencies without a broad grouping conversion. -Implementation review also retained the pandas compatibility comparisons, documented the offline invocation, and protected the disk-usage tests' original write errors when cleanup encounters an absent prefix. - -## File inventory - -```text -tests/__init__.py -tests/pyathena/__init__.py -tests/pyathena/aio/__init__.py -tests/pyathena/aio/arrow/__init__.py -tests/pyathena/aio/arrow/test_cursor.py -tests/pyathena/aio/conftest.py -tests/pyathena/aio/pandas/__init__.py -tests/pyathena/aio/pandas/test_cursor.py -tests/pyathena/aio/polars/__init__.py -tests/pyathena/aio/polars/test_cursor.py -tests/pyathena/aio/s3fs/__init__.py -tests/pyathena/aio/s3fs/test_cursor.py -tests/pyathena/aio/spark/__init__.py -tests/pyathena/aio/spark/test_cursor.py -tests/pyathena/aio/sqlalchemy/__init__.py -tests/pyathena/aio/sqlalchemy/test_base.py -tests/pyathena/aio/test_common.py -tests/pyathena/aio/test_connection.py -tests/pyathena/aio/test_cursor.py -tests/pyathena/aio/test_result_set.py -tests/pyathena/arrow/__init__.py -tests/pyathena/arrow/test_async_cursor.py -tests/pyathena/arrow/test_converter.py -tests/pyathena/arrow/test_cursor.py -tests/pyathena/arrow/test_result_set.py -tests/pyathena/arrow/test_util.py -tests/pyathena/conftest.py -tests/pyathena/filesystem/__init__.py -tests/pyathena/filesystem/test_init.py -tests/pyathena/filesystem/test_s3.py -tests/pyathena/filesystem/test_s3_async.py -tests/pyathena/filesystem/test_s3_core.py -tests/pyathena/filesystem/test_s3_errors.py -tests/pyathena/filesystem/test_s3_executor.py -tests/pyathena/filesystem/test_s3_object.py -tests/pyathena/filesystem/test_s3_path.py -tests/pyathena/pandas/__init__.py -tests/pyathena/pandas/test_async_cursor.py -tests/pyathena/pandas/test_converter.py -tests/pyathena/pandas/test_cursor.py -tests/pyathena/pandas/test_reader.py -tests/pyathena/pandas/test_result_set.py -tests/pyathena/pandas/test_util.py -tests/pyathena/polars/__init__.py -tests/pyathena/polars/test_async_cursor.py -tests/pyathena/polars/test_converter.py -tests/pyathena/polars/test_cursor.py -tests/pyathena/polars/test_result_set.py -tests/pyathena/s3fs/__init__.py -tests/pyathena/s3fs/test_async_cursor.py -tests/pyathena/s3fs/test_cursor.py -tests/pyathena/s3fs/test_reader.py -tests/pyathena/spark/__init__.py -tests/pyathena/spark/test_async_cursor.py -tests/pyathena/spark/test_common.py -tests/pyathena/spark/test_spark_cursor.py -tests/pyathena/sqlalchemy/__init__.py -tests/pyathena/sqlalchemy/test_array.py -tests/pyathena/sqlalchemy/test_base.py -tests/pyathena/sqlalchemy/test_compiler.py -tests/pyathena/sqlalchemy/test_map.py -tests/pyathena/sqlalchemy/test_struct.py -tests/pyathena/sqlalchemy/test_temporal.py -tests/pyathena/sqlalchemy/test_types.py -tests/pyathena/tables.py -tests/pyathena/test_async_cursor.py -tests/pyathena/test_connection.py -tests/pyathena/test_converter.py -tests/pyathena/test_cursor.py -tests/pyathena/test_formatter.py -tests/pyathena/test_glue.py -tests/pyathena/test_model.py -tests/pyathena/test_options.py -tests/pyathena/test_parser.py -tests/pyathena/test_result_set.py -tests/pyathena/test_util.py -tests/pyathena/util.py -tests/resources/queries/create_database.sql.jinja2 -tests/resources/queries/drop_database.sql.jinja2 -tests/sqlalchemy/__init__.py -tests/sqlalchemy/conftest.py -tests/sqlalchemy/profiles.txt -tests/sqlalchemy/test_suite.py -``` - -[sync-du]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/filesystem/test_s3.py#L4432-L4434 -[async-du]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/filesystem/test_s3_async.py#L1491-L1493 -[csv-helper]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L265-L300 -[csv-params]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L305-L389 -[csv-oracle]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L391-L423 -[polars-reader]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/polars/test_result_set.py#L259-L268 -[filesystem-identities]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L79-L80 -[unused-dtypes]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L439-L452 -[grouping-rule]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/AGENTS.md#L84-L85 -[parser-group]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/test_parser.py#L18-L36 -[utility-integration]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_util.py#L62-L70 -[converter-functions]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/test_converter.py#L19-L30 -[utility-writes]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_util.py#L451-L471 -[whole-read]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L52-L58 -[ddl-read]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_result_set.py#L139-L181 -[pandas-converter]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/pandas/test_cursor.py#L1945-L1971 -[s3fs-converter]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/s3fs/test_cursor.py#L543-L566 -[compiler-types]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/sqlalchemy/test_compiler.py#L621-L658 -[upstream-suite]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/sqlalchemy/test_suite.py#L35-L44 -[upstream-plugin]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/sqlalchemy/conftest.py#L11-L39 -[explicit-skips]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/sqlalchemy/test_suite.py#L974-L981 -[adapted-tests]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/NOTICE#L29-L36 -[cursor-lifetime]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/conftest.py#L225-L234 -[aio-lifetime]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/aio/conftest.py#L21-L32 -[arrow-binary]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/arrow/test_cursor.py#L38-L53 -[aio-arrow-binary]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/aio/arrow/test_cursor.py#L22-L36 -[session-hooks]: https://github.com/pyathena-dev/PyAthena/blob/200762088e7b0bac45054f22e32a16aa0ce95dbb/tests/pyathena/conftest.py#L15-L56 diff --git a/docs/testing.md b/docs/testing.md index 1b99a99d7..961f3c79e 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -195,14 +195,6 @@ A library comparison is useful when matching that library is the contract, such Express intentional differences directly rather than recreating the implementation in the expected-value builder. Retain dtype and schema checks, option contexts, resource cleanup, and equivalent synchronous and asynchronous scenarios where applicable. -The [test conventions audit](testing-audit.md) records the reviewed areas, source evidence, improvements, and intentional exceptions for issue #1079. - -```{toctree} -:hidden: - -testing-audit -``` - ## GitHub Actions The Test workflow runs for pull requests that change files other than `docs/` and Markdown. From 05c9e90c358ac1723ea594df1f5f5caac8c2b889 Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sun, 4 Oct 2026 22:04:55 +0900 Subject: [PATCH 5/6] Load offline test configuration from a dotenv file --- docs/testing.md | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index 961f3c79e..dbd0de4c8 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -134,21 +134,27 @@ A targeted run helps during development but does not replace other coverage requ ### Run self-contained tests offline The pandas and Polars result-set modules have self-contained tests that can run without AWS access when the session hooks are excluded. -After `just lint`, run: +Reuse the `.env` file from [AWS environment](#aws-environment) if it is already configured. +For an offline-only setup, create a gitignored `.env` file in the repository root with these placeholder values: + +```ini +AWS_DEFAULT_REGION=us-east-1 +AWS_ATHENA_S3_STAGING_DIR=s3://pyathena-offline-placeholder/ +AWS_ATHENA_WORKGROUP=offline +AWS_ATHENA_SPARK_WORKGROUP=offline +AWS_EC2_METADATA_DISABLED=true +``` + +After `just lint`, load `.env` and run: ```bash -env AWS_DEFAULT_REGION=us-east-1 \ - AWS_ATHENA_S3_STAGING_DIR=s3://pyathena-offline-placeholder/ \ - AWS_ATHENA_WORKGROUP=offline \ - AWS_ATHENA_SPARK_WORKGROUP=offline \ - AWS_EC2_METADATA_DISABLED=true \ - uv run pytest --noconftest -p no:rerunfailures -q \ +uv run --env-file .env pytest --noconftest -p no:rerunfailures -q \ tests/pyathena/pandas/test_result_set.py \ tests/pyathena/polars/test_result_set.py ``` -The four placeholder AWS configuration values satisfy `tests/__init__.py`, which pytest still imports with `--noconftest`. -Disabling EC2 metadata prevents implicit credential lookup through that service. +The four AWS configuration values are required by `tests/__init__.py`, which pytest still imports with `--noconftest`. +The offline-only `.env` example disables EC2 metadata to prevent implicit credential lookup through that service. `--noconftest` excludes the AWS session hooks and fixtures; disabling the rerun plugin also avoids its local socket setup in restricted environments. Use this invocation only for self-contained modules; integration tests need their normal fixtures and a real AWS environment. From e72974b4823aa83873784dbc3345bb1168ac449b Mon Sep 17 00:00:00 2001 From: laughingman7743 Date: Sun, 4 Oct 2026 22:06:59 +0900 Subject: [PATCH 6/6] Avoid an unresolved reference in the offline setup guide --- docs/testing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/testing.md b/docs/testing.md index dbd0de4c8..db559fcc6 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -134,7 +134,7 @@ A targeted run helps during development but does not replace other coverage requ ### Run self-contained tests offline The pandas and Polars result-set modules have self-contained tests that can run without AWS access when the session hooks are excluded. -Reuse the `.env` file from [AWS environment](#aws-environment) if it is already configured. +Reuse the `.env` file described in the AWS environment section if it is already configured. For an offline-only setup, create a gitignored `.env` file in the repository root with these placeholder values: ```ini