Prepare v1.7.2: fix file naming, improve error messages - #62
Merged
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Files were named output.csv_0 instead of output_0.csv when using the files: N config option. Split the extension from the base name before inserting the file index suffix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Provider names are already resolved case-insensitively via to_lowercase() at the entry point. Add an explicit test covering mixed-case and uppercase variants of all provider families. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sub-builders returned empty error strings, and config.rs double-wrapped
them into nested BadYAMLFormat("BadYAMLFormat(\"BadYAMLFormat(\"\")\")")
messages. Fix both issues:
- Each sub-builder now reports the unrecognized segment and lists valid
options (e.g. "Unknown provider: random.float. Expected one of:
random.bool, random.date.*, random.number.*, random.string.*")
- config.rs propagates provider errors directly instead of re-wrapping
- FakeLakeError::Display now formats human-readable messages instead of
using Debug output
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…iant Replace duplicated "Unknown provider: ... Expected one of: ..." format strings across all builders with a structured FakeLakeError::UnknownProvider variant that holds the wrong provider name and a static list of valid options. Each builder declares its AVAILABLE list as a const and delegates formatting to the Display impl. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Prepares the v1.7.2 release by correcting multi-file output naming, improving provider-resolution error reporting, and documenting provider lookup behavior (case-insensitive) via tests.
Changes:
- Bump crate version to
1.7.2. - Adjust multi-file output naming to insert the index before the extension (e.g.
output_0.csv). - Add
UnknownProvidererror variant + improvedDisplay, propagate errors without double-wrapping, and add case-insensitive provider resolution tests.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test.rs |
Updates integration test expectations/cleanup for new multi-file Parquet naming. |
src/generate/output_format.rs |
Implements index-before-extension naming for multi-file generation. |
src/errors.rs |
Adds UnknownProvider and improves human-readable error formatting. |
src/config.rs |
Stops re-wrapping provider-builder errors at the column level. |
src/providers/provider.rs |
Updates unknown-provider handling to include valid options; adds case-insensitive provider resolution test. |
src/providers/random/builder.rs |
Improves unknown-provider errors for random.* root selection. |
src/providers/random/date/builder.rs |
Improves unknown-provider errors for date sub-providers. |
src/providers/random/number/builder.rs |
Improves unknown-provider errors for number sub-providers. |
src/providers/random/string/builder.rs |
Improves unknown-provider errors for string sub-providers. |
src/providers/person/builder.rs |
Improves unknown-provider errors for person sub-providers. |
src/providers/increment/builder.rs |
Improves unknown-provider errors for increment sub-providers. |
src/providers/constant/builder.rs |
Improves unknown-provider errors for constant sub-providers. |
Cargo.toml |
Version bump to 1.7.2. |
Cargo.lock |
Lockfile updated for version bump. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The unknown_provider function now accepts a prefix and an Option<&str>
segment, and assembles the full provider name internally. This removes
the duplicated format!("prefix.{}", other.unwrap_or("<missing>"))
pattern from every builder call site.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address Copilot review: replace rfind with strip_suffix to avoid truncating filenames that contain the extension mid-string. Also add an integration test verifying multi-file indexed naming for CSV and JSON formats, not just Parquet. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
output.csv_0instead ofoutput_0.csvwhen usingfiles: N. The index suffix is now inserted before the extension for all formats (CSV, JSON, Parquet)RANDOM.NUMBER.F64orPerson.Fnamealready resolve correctly — added explicit tests documenting this behaviorBadYAMLFormat("BadYAMLFormat(\"\")")messages. Now each builder reports the unrecognized segment and lists valid options (e.g.Unknown provider: random.float. Expected one of: random.bool, random.date.*, random.number.*, random.string.*). Also fixedFakeLakeError::Displayto produce human-readable output instead of Debug formattingTest plan
given_seeded_multifile_parquet_should_be_reproducibleupdated for new naming)cargo clippy --all-targets --all-features -- -D warningscleanfiles: 3producesoutput_0.csv,output_1.csv,output_2.csvrandom.floatnow shows helpful error with suggestions🤖 Generated with Claude Code