Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: unify json & parquet checkpoint creation methods in the MockTable #687

Closed
sebastiantia opened this issue Feb 10, 2025 · 1 comment
Closed
Labels
enhancement New feature or request

Comments

@sebastiantia
Copy link
Collaborator

Please describe why this is necessary.

Our current test suite uses two distinct methods to create checkpoints in MockTable:
The method parquet_checkpoint accepts engine data
The method json_checkpoint accepta actions to create a JSON checkpoint file.

This inconsistency causes the test cases—such as in test_create_checkpoint_stream_reads_parquet_checkpoint_batch versus test_create_checkpoint_stream_reads_json_checkpoint_batch — to look very different, which increases maintenance overhead and introduces potential discrepancies in handling different checkpoint formats.

Describe the functionality you are proposing.

Create a single method (e.g. create_checkpoint) for the MockTable struct test utility that accepts checkpoint actions as engine data. This method should internally handle the conversion for both JSON and Parquet formats, dependent on a passed parameter.

Instead of having one function that takes a batch of engine data and another that takes actions, standardize on one as the input. This change will streamline the test setup and ensure consistency across both checkpoint types.

Update tests accordingly

Additional context

No response

@sebastiantia sebastiantia added the enhancement New feature or request label Feb 10, 2025
@sebastiantia sebastiantia changed the title test: unify MockTable test utility for mock checkpoint creation (JSON & Parquet) test: unify checkpoint creation methods for json & parquet in the MockTable Feb 10, 2025
@sebastiantia sebastiantia changed the title test: unify checkpoint creation methods for json & parquet in the MockTable test: unify json & parquet checkpoint creation methods in the MockTable Feb 10, 2025
@sebastiantia
Copy link
Collaborator Author

We are moving away from adding the checkpointing functionality in the mock table until a broader conversation is held as to whether or not we want to continue with the MockTable testing infrastructure

@sebastiantia sebastiantia closed this as not planned Won't fix, can't repro, duplicate, stale Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant