pitrou commented on a change in pull request #7803:
URL: https://github.com/apache/arrow/pull/7803#discussion_r468605319



##########
File path: cpp/src/arrow/filesystem/s3fs_test.cc
##########
@@ -197,6 +201,44 @@ TEST(S3Options, FromUri) {
   ASSERT_RAISES(Invalid, S3Options::FromUri("s3:///foo/bar/", &path));
 }
 
+TEST(S3Options, FromAccessKey) {
+  S3Options options;
+
+  // session token is optional and should default to empty string
+  options = S3Options::FromAccessKey("access", "secret");
+  ASSERT_EQ(options.GetAccessKey(), "access");
+  ASSERT_EQ(options.GetSecretKey(), "secret");
+  ASSERT_EQ(options.GetSessionToken(), "");
+
+  options = S3Options::FromAccessKey("access", "secret", "token");
+  ASSERT_EQ(options.GetAccessKey(), "access");
+  ASSERT_EQ(options.GetSecretKey(), "secret");
+  ASSERT_EQ(options.GetSessionToken(), "token");
+}
+
+TEST(S3Options, FromAssumeRole) {
+  S3Options options;
+
+  // we set this envvar to speed up tests by ensuring
+  // DefaultAWSCredentialsProviderChain does not query (inaccessible)
+  // EC2 metadata endpoint
+  ASSERT_OK(SetEnvVar("AWS_EC2_METADATA_DISABLED", "true"));

Review comment:
       So, the right way to do this would be to create a test fixture:
   ```c++
   class S3OptionsTest : public ::testing::Test {
    public:
     void SetUp() {
       ASSERT_OK(SetEnvVar("AWS_EC2_METADATA_DISABLED", "true"));
     }
     void TearDown() {
       ASSERT_OK(DelEnvVar("AWS_EC2_METADATA_DISABLED"));
     }
   };
   ```
   And then to replace every `TEST(S3Options, ...)` with `TEST_F(S3OptionsTest, 
...)`.




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to