You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/06/08 21:36:46 UTC
[GitHub] [arrow-rs] tustvold commented on a diff in pull request #1822: Test more feature flag combinations in CI (#1630)
tustvold commented on code in PR #1822:
URL: https://github.com/apache/arrow-rs/pull/1822#discussion_r892883612
##########
.github/workflows/rust.yml:
##########
@@ -108,31 +108,39 @@ jobs:
# run tests on all workspace members with default feature list
cargo test
-
- # Switch to arrow crate
- cd arrow
- # re-run tests on arrow crate to ensure
- # all arrays are created correctly
- cargo test --features=force_validate
- cargo test --features=prettyprint
- # run test on arrow crate with minimal set of features
- cargo test --no-default-features
+
+ # re-run tests on arrow crate with all supported features
+ cargo test -p arrow --features=force_validate,prettyprint
Review Comment:
I merged these two test runs together as I couldn't see an obvious reason to separate them, and this will make CI slightly faster
##########
.github/workflows/rust.yml:
##########
@@ -108,31 +108,39 @@ jobs:
# run tests on all workspace members with default feature list
cargo test
-
- # Switch to arrow crate
- cd arrow
- # re-run tests on arrow crate to ensure
- # all arrays are created correctly
- cargo test --features=force_validate
- cargo test --features=prettyprint
- # run test on arrow crate with minimal set of features
- cargo test --no-default-features
+
+ # re-run tests on arrow crate with all supported features
+ cargo test -p arrow --features=force_validate,prettyprint
+
+ # Test arrow examples
cargo run --example builders
cargo run --example dynamic_types
cargo run --example read_csv
cargo run --example read_csv_infer_schema
- cargo check --no-default-features
+
Review Comment:
I moved the `cargo check` logic into here as opposed to a separate job so it can benefit from the compilation already performed above.
##########
parquet/src/errors.rs:
##########
@@ -135,6 +135,7 @@ macro_rules! eof_err {
($fmt:expr, $($args:expr),*) => (ParquetError::EOF(format!($fmt, $($args),*)));
}
+#[cfg(any(feature = "arrow", test))]
Review Comment:
This removes a warning
##########
Cargo.toml:
##########
@@ -24,6 +24,7 @@ members = [
"arrow-flight",
"integration-testing",
]
+resolver = "2"
Review Comment:
Enable new resolver
##########
parquet/src/record/api.rs:
##########
@@ -27,7 +27,7 @@ use crate::data_type::{ByteArray, Decimal, Int96};
use crate::errors::{ParquetError, Result};
use crate::schema::types::ColumnDescPtr;
-#[cfg(feature = "cli")]
+#[cfg(any(feature = "cli", test))]
Review Comment:
Drive by refactor to make consistent with rest of crate which tests everything
##########
.github/workflows/rust.yml:
##########
@@ -108,31 +108,42 @@ jobs:
# run tests on all workspace members with default feature list
cargo test
-
- # Switch to arrow crate
- cd arrow
- # re-run tests on arrow crate to ensure
- # all arrays are created correctly
- cargo test --features=force_validate
- cargo test --features=prettyprint
- # run test on arrow crate with minimal set of features
- cargo test --no-default-features
+
+ # re-run tests on arrow crate with all supported features
+ cargo test -p arrow --features=force_validate,prettyprint
+
+ # Test arrow examples
cargo run --example builders
cargo run --example dynamic_types
cargo run --example read_csv
cargo run --example read_csv_infer_schema
- cargo check --no-default-features
+
+ # Test compilation of arrow library crate with different feature combinations
+ cargo check -p arrow
+ cargo check -p arrow --no-default-features
+
+ # Test compilation of arrow targets with different feature combinations
+ cargo check -p arrow --all-targets
+ cargo check -p arrow --no-default-features --all-targets
- # Switch to parquet crate
- cd ../parquet
- # re-run tests on parquet crate with async feature enabled
- cargo test --features=async
- cargo check --no-default-features
+ # re-run tests on arrow-flight with all features
+ cargo test -p arrow-flight --all-features
- # Switch to arrow-flight
- cd ../arrow-flight
- cargo test --features=flight-sql-experimental
- cargo check --no-default-features
+ # re-run tests on parquet crate with all features
+ cargo test -p parquet --all-features
+
+ # Test compilation of parquet library crate with different feature combinations
+ cargo check -p parquet
+ cargo check -p parquet --no-default-features
+ cargo check -p parquet --no-default-features --features arrow
+
+ # Test compilation of parquet targets with different feature combinations
+ cargo check -p parquet --all-targets
+ cargo check -p parquet --no-default-features --all-targets
+ cargo check -p parquet --no-default-features --features arrow --all-targets
+
+ # Test compilation of parquet_derive macro with different feature combinations
+ cargo check -p parquet_derive
Review Comment:
To be completely honest I'm not sure why we were testing this, parquet_derive doesn't have any dev-dependencies or feature flags, but I guess it can't hurt to be thorough
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org