Skip to content

test: follow up FileSystemSeedReader coverage cleanup#432

Open
eric-tramel wants to merge 1 commit intomainfrom
pr-424-filesystem-seed-reader-test-followups
Open

test: follow up FileSystemSeedReader coverage cleanup#432
eric-tramel wants to merge 1 commit intomainfrom
pr-424-filesystem-seed-reader-test-followups

Conversation

@eric-tramel
Copy link
Contributor

Summary

  • consolidate the shared line-fanout FileSystemSeedReader test helper so engine and interface tests use the same implementation
  • collapse duplicate engine-only hydration test readers and reuse a shared alpha/beta fixture to trim repeated setup
  • add the missing follow-up coverage for None invalid returns and the full-output all-empty-fanout guard

Context

Testing

  • uv run ruff check packages/data-designer-engine/src/data_designer/engine/testing/seed_readers.py packages/data-designer-engine/tests/engine/resources/test_seed_reader.py packages/data-designer/tests/interface/test_data_designer.py
  • uv run pytest packages/data-designer-engine/tests/engine/resources/test_seed_reader.py packages/data-designer/tests/interface/test_data_designer.py

@eric-tramel eric-tramel requested a review from a team as a code owner March 18, 2026 01:01
@eric-tramel eric-tramel self-assigned this Mar 18, 2026
@eric-tramel eric-tramel added the chore 🧹 label Mar 18, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 18, 2026

Greptile Summary

This PR is a clean follow-up cleanup to #424 that consolidates duplicate FanoutDirectorySeedReader-style test helpers into a single, shared LineFanoutDirectorySeedReader living in data_designer.engine.testing, and merges the InvalidHydrationReturnSeedReader / SchemaMismatchFanoutSeedReader pair into a single ConfigurableHydrationDirectorySeedReader. It also adds two previously missing coverage cases: the None return path in test_filesystem_seed_reader_rejects_invalid_hydrate_row_returns and the new test_filesystem_seed_reader_full_output_raises_when_all_manifest_rows_fan_out_to_empty guard test.

  • seed_readers.py is a new shared testing helper module; LineFanoutDirectorySeedReader is correctly re-exported from __init__.py, resolving the inconsistency raised in the prior review.
  • Column ordering with include_file_name=True (relative_path → file_name → line_index → line) exactly matches the old FanoutDirectorySeedReader class attribute, so no behaviour change is introduced.
  • include_file_name=False (default) preserves the FanoutCustomDirectorySeedReader column schema used in the interface tests.
  • The new write_alpha_beta_text_files pytest fixture neatly reduces per-test file setup boilerplate across four fanout-related engine tests.
  • All changes are test-scoped; no production code is modified.

Confidence Score: 5/5

  • This PR is safe to merge — it is a pure test-layer refactoring with no production code changes.
  • All changes are confined to test utilities and test files. The consolidated LineFanoutDirectorySeedReader correctly preserves both the include_file_name=True and include_file_name=False column schemas, and the ConfigurableHydrationDirectorySeedReader is a straightforward union of two previously separate reader stubs. The new None-return test case and the all-empty-fanout guard test exercise real code paths in the engine. No regressions are expected.
  • No files require special attention.

Important Files Changed

Filename Overview
packages/data-designer-engine/src/data_designer/engine/testing/seed_readers.py New file introducing the shared LineFanoutDirectorySeedReader helper, consolidating two previously duplicate implementations from the engine and interface test files. Logic is correct; instance-level output_columns setup works at runtime.
packages/data-designer-engine/src/data_designer/engine/testing/init.py Adds the LineFanoutDirectorySeedReader import and re-export to address the previous review comment about inconsistency with other test helpers.
packages/data-designer-engine/tests/engine/resources/test_seed_reader.py Removes FanoutDirectorySeedReader, InvalidHydrationReturnSeedReader, and SchemaMismatchFanoutSeedReader in favour of the shared LineFanoutDirectorySeedReader and a new ConfigurableHydrationDirectorySeedReader. Adds write_alpha_beta_text_files fixture, None-return parametrize case, and the all-empty-fanout full-output guard test.
packages/data-designer/tests/interface/test_data_designer.py Replaces the local FanoutCustomDirectorySeedReader with the canonical LineFanoutDirectorySeedReader from the testing package; no behavioural changes, column schema is identical.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["LineFanoutDirectorySeedReader\n(engine/testing/seed_readers.py)"]
    B["include_file_name=True\noutput_columns: relative_path, file_name, line_index, line"]
    C["include_file_name=False\noutput_columns: relative_path, line_index, line"]

    A --> B
    A --> C

    B --> D["test_seed_reader.py\n(engine tests)"]
    C --> E["test_data_designer.py\n(interface tests)"]

    F["ConfigurableHydrationDirectorySeedReader"]
    F --> G["hydrated_return=None → NoneType error"]
    F --> H["hydrated_return=scalar → scalar type error"]
    F --> I["hydrated_return=list with wrong schema → column mismatch error"]
    F --> D
Loading

Last reviewed commit: "test: trim FileSyste..."

@eric-tramel eric-tramel force-pushed the pr-424-filesystem-seed-reader-test-followups branch from e411688 to 33f4392 Compare March 18, 2026 01:09
@eric-tramel eric-tramel force-pushed the pr-424-filesystem-seed-reader-test-followups branch from 33f4392 to 26276e0 Compare March 18, 2026 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore 🧹

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant