Skip to content

test: drive stdin store reuse through get_or_create#23061

Open
huan233usc wants to merge 1 commit into
apache:mainfrom
huan233usc:cli-stdin-reuse-test
Open

test: drive stdin store reuse through get_or_create#23061
huan233usc wants to merge 1 commit into
apache:mainfrom
huan233usc:cli-stdin-reuse-test

Conversation

@huan233usc

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Follow-up to #22839 (stdin support in datafusion-cli), addressing post-merge review feedback from @alamb. No separate issue.

Rationale for this change

In the review of #22839, @alamb noted the reuses_buffered_stdin_store test would be clearer if it used the same API to create and re-fetch the object store:

this test would be clearer for me if it used the same API to create and recreate the object store -- aka StdinUtils::get_or_create rather than StdinUtils::in_memory_object_store

The test built the buffered store via StdinUtils::in_memory_object_store but re-fetched it via StdinUtils::get_or_create, which obscured what "reuse" actually guarantees.

What changes are included in this PR?

Refactor reuses_buffered_stdin_store so the only StdinUtils API it exercises is get_or_create:

  • Seed the registry with a plain InMemory store. The genuine first stdin read happens inside get_or_createobject_store, which consumes the real process stdin and so cannot be driven from a unit test; the comment now explains this.
  • Assert via Arc::ptr_eq that get_or_create hands back that exact store rather than rebuilding it — a stronger and clearer statement of the reuse invariant than comparing bytes alone.

Test-only change; no production behavior change.

Are these changes tested?

Yes — this is a test change. Verified locally:

cargo test -p datafusion-cli --lib object_storage::stdin
...
test object_storage::stdin::tests::reuses_buffered_stdin_store ... ok
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 62 filtered out

Are there any user-facing changes?

No.

Address review feedback on apache#22839: the reuses_buffered_stdin_store test
created the store one way (StdinUtils::in_memory_object_store) and
re-fetched it another (get_or_create), which obscured what reuse means.

Seed the registry with a plain InMemory store (the genuine first read
goes through get_or_create -> object_store, which consumes real process
stdin and can't be driven from a unit test), then assert get_or_create
hands back that exact store via Arc::ptr_eq rather than rebuilding it.
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.

1 participant