Skip to content

GH-3289: Test read commands through CLI arguments - #3858

Open
zhangxinyao88 wants to merge 2 commits into
apache:masterfrom
zhangxinyao88:codex/parquet-3289-read-cli-tests
Open

zhangxinyao88 wants to merge 2 commits into
apache:masterfrom
zhangxinyao88:codex/parquet-3289-read-cli-tests

Conversation

@zhangxinyao88

Copy link
Copy Markdown
Contributor

Rationale for this change

Continue #3289 with CLI coverage for head, cat, and scan. Existing command tests mostly set fields directly and check return codes, leaving argument parsing and observable results untested.

What changes are included in this PR?

Add six tests using the existing CLI helper and deterministic Parquet files:

  • head defaults to ten records and honors --num-records.
  • cat prints all records and honors --columns.
  • scan counts records across multiple input files and rejects a missing column.

Record output is parsed and compared by field and value. Scan assertions check counts and the missing-field diagnostic without depending on elapsed time or complete log output.

Are these changes tested?

./mvnw -B -ntp -pl parquet-cli spotless:check test: 87 passed, 2 skipped, including all six new tests. Spotless and license checks passed.

Are there any user-facing changes?

No. Tests only.

@divjotarora divjotarora left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @zhangxinyao88!

.named("unused")
.named("record");
SimpleGroupFactory groups = new SimpleGroupFactory(schema);
try (ParquetWriter<Group> writer = ExampleParquetWriter.builder(new org.apache.hadoop.fs.Path(path.toUri()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Add an import for org.apache.hadoop.fs.Path?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Add an import for org.apache.hadoop.fs.Path?

Added the Hadoop Path import in d943e37.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @zhangxinyao88, I missed the fact that the test also needs java.nio.file.Path and this one is used multiple times while org.apache.hadoop.fs.Path is only used once. I would actually recommend reverting the change back to importing java.nio.file.Path and using org.apache.hadoop.fs.Path fully-qualified because of this. Sorry for the churn!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants