[GitHub] [arrow-rs] tustvold commented on a diff in pull request #4628: Use Config System for Object Store Integration Tests

["tustvold (via GitHub)" <gi...@apache.org>]

tustvold commented on code in PR #4628:
URL: https://github.com/apache/arrow-rs/pull/4628#discussion_r1281748833


##########
object_store/src/aws/mod.rs:
##########
@@ -1067,153 +1067,9 @@ mod tests {
     };
     use bytes::Bytes;
     use std::collections::HashMap;
-    use std::env;
 
     const NON_EXISTENT_NAME: &str = "nonexistentname";
 
-    // Helper macro to skip tests if TEST_INTEGRATION and the AWS
-    // environment variables are not set. Returns a configured
-    // AmazonS3Builder
-    macro_rules! maybe_skip_integration {
-        () => {{
-            dotenv::dotenv().ok();
-
-            let required_vars = [
-                "OBJECT_STORE_AWS_DEFAULT_REGION",
-                "OBJECT_STORE_BUCKET",
-                "OBJECT_STORE_AWS_ACCESS_KEY_ID",
-                "OBJECT_STORE_AWS_SECRET_ACCESS_KEY",
-            ];
-            let unset_vars: Vec<_> = required_vars
-                .iter()
-                .filter_map(|&name| match env::var(name) {
-                    Ok(_) => None,
-                    Err(_) => Some(name),
-                })
-                .collect();
-            let unset_var_names = unset_vars.join(", ");
-
-            let force = env::var("TEST_INTEGRATION");
-
-            if force.is_ok() && !unset_var_names.is_empty() {
-                panic!(
-                    "TEST_INTEGRATION is set, \
-                            but variable(s) {} need to be set",
-                    unset_var_names
-                );
-            } else if force.is_err() {
-                eprintln!(
-                    "skipping AWS integration test - set {}TEST_INTEGRATION to run",
-                    if unset_var_names.is_empty() {
-                        String::new()
-                    } else {
-                        format!("{} and ", unset_var_names)
-                    }
-                );
-                return;
-            } else {
-                let config = AmazonS3Builder::new()
-                    .with_access_key_id(
-                        env::var("OBJECT_STORE_AWS_ACCESS_KEY_ID")
-                            .expect("already checked OBJECT_STORE_AWS_ACCESS_KEY_ID"),
-                    )
-                    .with_secret_access_key(
-                        env::var("OBJECT_STORE_AWS_SECRET_ACCESS_KEY")
-                            .expect("already checked OBJECT_STORE_AWS_SECRET_ACCESS_KEY"),
-                    )
-                    .with_region(
-                        env::var("OBJECT_STORE_AWS_DEFAULT_REGION")
-                            .expect("already checked OBJECT_STORE_AWS_DEFAULT_REGION"),
-                    )
-                    .with_bucket_name(
-                        env::var("OBJECT_STORE_BUCKET")
-                            .expect("already checked OBJECT_STORE_BUCKET"),
-                    )
-                    .with_allow_http(true);
-
-                let config = if let Ok(endpoint) = env::var("OBJECT_STORE_AWS_ENDPOINT") {
-                    config.with_endpoint(endpoint)
-                } else {
-                    config
-                };
-
-                let config = if let Ok(token) = env::var("OBJECT_STORE_AWS_SESSION_TOKEN")
-                {
-                    config.with_token(token)
-                } else {
-                    config
-                };
-
-                let config = if let Ok(virtual_hosted_style_request) =
-                    env::var("OBJECT_STORE_VIRTUAL_HOSTED_STYLE_REQUEST")
-                {
-                    config.with_virtual_hosted_style_request(
-                        virtual_hosted_style_request.trim().parse().unwrap(),
-                    )
-                } else {
-                    config
-                };
-
-                config
-            }
-        }};
-    }
-
-    #[test]
-    fn s3_test_config_from_env() {

Review Comment:
   This test is no longer necessary as we test that from_env is hooked up as part of all tests, and s3_test_config_from_map below tests the various config keys



-- 
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