Move Parquet encryption tests into the arrow_reader integration tests #7279
Conversation
adamreeve
force-pushed
the
encryption-test-tidy
branch
from
2a0c109 to
9ac075b
Compare
parquet/tests/arrow_reader/mod.rs
Outdated
| mod encryption; | ||
| #[cfg(all(feature = "encryption", feature = "async"))] | ||
| mod encryption_async; | ||
| mod encryption_common; |
There was a problem hiding this comment.
Nit: how about read_encrypted_with_plaintext_footer or encryption_agnostic? encryption_common slightly implies some sort of shared utility and made me wonder why it's not behind #[cfg(feature = "encryption")]. Not a big deal, feel free to ignore.
There was a problem hiding this comment.
Yeah encryption_agnostic sounds better, I couldn't think of a good name for this.
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @adamreeve -- this is great (and thank you @rok for the ping)
I also double checked that the tests are run as part of CI: https://github.com/apache/arrow-rs/actions/runs/13826035766/job/38681041589?pr=7279
| assert_eq!(file_metadata.schema_descr().num_columns(), 8); | ||
|
|
||
| metadata.metadata.row_groups().iter().for_each(|rg| { | ||
| metadata.row_groups().iter().for_each(|rg| { |
|
Since these are tests only changes I am going to merge it in to avoid conflicts and get us ready for the next round |
|
Thanks agian! |
3 participants
Rationale for this change
See #6637 (comment)
This ensures that the tests are built with the same visibility restrictions as user programs, and reduces the need for
#[cfg(feature = "encryption")]on all tests.What changes are included in this PR?
parquet/tests/arrow_readerverify_encryption_test_datafunction for the test that usesobject_store.Are there any user-facing changes?
No