Skip to content

Audit test conventions and identify focused consistency improvements #1079

Closed

Description

@laughingman7743

Use case

While reviewing the tests around #1073 and #1075, we noticed differences in test organization and data preparation that deserve a separate audit.
Keeping conventions clear makes regression coverage easier to review and reduces repeated setup and complex expected-value construction.

One starting point is tests/pyathena/pandas/test_result_set.py, lines 223–468 at 707d6488:

  • Parameter definitions repeatedly call a helper that patches result-set properties and builds CSV options during module import/test collection.
  • The pandas comparison test constructs its expected result using two parsers, a loop, and conditional adjustments, making each case's intended contract harder to follow.
  • Standalone CSV-helper tests coexist with class-grouped result-set tests.

The class/function distinction can be appropriate: AGENTS.md calls for class-based integration tests using cursor/engine fixtures and standalone tests for pure logic.
These examples are starting points for investigation, not a finding that every standalone test or computed expected value needs changing.
The rest of the test suite has not been audited as part of this follow-up.

Proposed change

Run a separate audit of project-owned tests under tests/, including their conftest.py files and local test helpers.

  1. Inventory the conventions used by integration tests, pure-logic unit tests, and the SQLAlchemy compliance suites.
    Record fixture use, class/function organization, parametrization, helper placement, and equivalent synchronous/asynchronous coverage where relevant.
    Account for framework requirements and the provenance of adapted tests.
  2. Compare these patterns with AGENTS.md and representative existing tests.
    Where guidance is ambiguous or differs from a valid established pattern, propose a focused clarification before applying a broad conversion.
  3. Check data preparation and assertions as well as test grouping:
    • Data-driven cases should make inputs, options, and expected behavior easy to identify.
    • Examine mock/setup work performed during collection, repeated helper calls, and redundant setup.
    • Check whether expected-value construction obscures the contract or repeats the implementation being tested.
    • Check fixture reuse, readable parameter IDs, and explanation style within each module.
  4. Produce a source-based report with exact file:line references, the relevant convention or rationale, the maintenance or coverage impact, and a proposed disposition.
    Distinguish actionable inconsistencies from intentional differences.
  5. Prioritize focused, behavior-preserving follow-up PRs from that report.
    Preserve meaningful assertions, parameter coverage, fixture/resource lifetimes, and test selection when reorganizing tests.

This work is separate from the DDL/PyArrow behavior fix in #1073 and #1075 and is intended for a separate session.

Expected deliverables:

  • An inventory of the reviewed test areas and any unreviewed areas.
  • Evidence-backed findings and intentional exceptions, with prioritized follow-ups.
  • Any agreed clarification of the test conventions before implementing widespread changes.

Validation plan (if implementing)

The initial audit is static source inspection and requires no AWS resources.
Record its coverage and limitations without claiming runtime validation.

For subsequent code changes, run just format and just lint, compare test collection and case coverage before and after the change, and run the affected tests.
Preserve dtype/schema inputs, option-context behavior, and observable assertions when moving data preparation.
Use an offline invocation for self-contained unit tests; validate affected integration behavior in a configured AWS environment and serialize live validation with other local and CI runs.
Each follow-up PR should record the tested revision, commands, results, and skipped coverage and follow the repository's review and CI workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions