Skip to content

Commit c75b82d

Browse files
authored
GH-40410: [C++] Skip only s3fs-tests and s3fs-module-tests that require MinIO if MinIO is not available (#50215)
### Rationale for this change There are lots of S3 tests that doesn't require MinIO which we are not being exercised on CI because we skip the full test suite. We should skip only the tests that use MinIO if MinIO is not found but not the ones that doesn't require MinIO. ### What changes are included in this PR? - Remove all occurrences of `exclude_tests="arrow-s3fs-test"` - Add MinIO to path for macOS cpp.yml builds so MinIO tests are exercised - Add new `MinioTestEnvironment::IsAvailable` to validate whether MinIO is available or not as part of the test suite - `GTEST_SKIP` tests that require MinIO ### Are these changes tested? Yes via CI and existing tests. I've validated this locally both ways: without MinIO the non-MinIO tests run and the MinIO dependant ones are skipped, with MinIO on PATH the full suite runs and passes ### Are there any user-facing changes? No * GitHub Issue: #40410 Authored-by: Raúl Cumplido <raulcumplido@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
1 parent 1926639 commit c75b82d

7 files changed

Lines changed: 47 additions & 21 deletions

File tree

.github/workflows/cpp.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,8 @@ jobs:
293293
- name: Test
294294
shell: bash
295295
run: |
296+
# MinIO is installed on ${ARROW_HOME}/bin
297+
export PATH="${ARROW_HOME}/bin:${PATH}"
296298
sudo sysctl -w kern.coredump=1
297299
sudo sysctl -w kern.corefile=/tmp/core.%N.%P
298300
ulimit -c unlimited # must enable within the same shell

.github/workflows/cpp_extra.yml

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -340,10 +340,8 @@ jobs:
340340
ARROW_TEST_DATA: ${{ github.workspace }}/testing/data
341341
PARQUET_TEST_DATA: ${{ github.workspace }}/cpp/submodules/parquet-testing/data
342342
run: |
343-
# MinIO is required
344-
exclude_tests="arrow-s3fs-test"
345343
# unstable
346-
exclude_tests="${exclude_tests}|arrow-acero-asof-join-node-test"
344+
exclude_tests="arrow-acero-asof-join-node-test"
347345
exclude_tests="${exclude_tests}|arrow-acero-hash-join-node-test"
348346
ctest \
349347
--exclude-regex "${exclude_tests}" \

ci/scripts/cpp_test.sh

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,17 +50,12 @@ fi
5050
if ! type storage-testbench >/dev/null 2>&1; then
5151
exclude_tests+=("arrow-gcsfs-test")
5252
fi
53-
if ! type minio >/dev/null 2>&1; then
54-
exclude_tests+=("arrow-s3fs-test")
55-
fi
5653
case "$(uname)" in
5754
Linux)
5855
n_jobs=$(nproc)
5956
;;
6057
Darwin)
6158
n_jobs=$(sysctl -n hw.ncpu)
62-
# TODO: https://github.com/apache/arrow/issues/40410
63-
exclude_tests+=("arrow-s3fs-test")
6459
;;
6560
MINGW*)
6661
n_jobs=${NUMBER_OF_PROCESSORS:-1}

cpp/src/arrow/filesystem/s3_test_util.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,19 @@ void MinioTestEnvironment::SetUp() {
192192
MakeReadaheadGenerator(std::move(impl_->server_generator_), pool->GetCapacity());
193193
}
194194

195+
bool MinioTestEnvironment::IsAvailable() {
196+
if (!available_.has_value()) {
197+
// If external S3-compatible service is configured there's no need to check for Minio
198+
available_ = std::getenv(kEnvConnectString) && std::getenv(kEnvAccessKey) &&
199+
std::getenv(kEnvSecretKey);
200+
if (!available_.value()) {
201+
::arrow::util::Process process;
202+
available_ = process.SetExecutable(kMinioExecutableName).ok();
203+
}
204+
}
205+
return available_.value();
206+
}
207+
195208
Result<std::shared_ptr<MinioTestServer>> MinioTestEnvironment::GetOneServer() {
196209
return impl_->server_generator_().result();
197210
}

cpp/src/arrow/filesystem/s3_test_util.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#pragma once
1919

2020
#include <memory>
21+
#include <optional>
2122
#include <string>
2223
#include <utility>
2324

@@ -74,6 +75,11 @@ class MinioTestEnvironment : public ::testing::Environment {
7475

7576
Result<std::shared_ptr<MinioTestServer>> GetOneServer();
7677

78+
bool IsAvailable();
79+
80+
private:
81+
std::optional<bool> available_;
82+
7783
protected:
7884
struct Impl;
7985
std::unique_ptr<Impl> impl_;

cpp/src/arrow/filesystem/s3fs_module_test.cc

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,17 @@ MinioTestEnvironment* GetMinioEnv() {
5555
return ::arrow::internal::checked_cast<MinioTestEnvironment*>(minio_env);
5656
}
5757

58+
class S3ModuleTest : public ::testing::Test {
59+
protected:
60+
void SetUp() override {
61+
if (!GetMinioEnv()->IsAvailable()) {
62+
GTEST_SKIP() << "Minio executable not found, skipping tests";
63+
}
64+
ASSERT_OK_AND_ASSIGN(minio_, GetMinioEnv()->GetOneServer());
65+
}
66+
std::shared_ptr<MinioTestServer> minio_;
67+
};
68+
5869
class RegistrationTestEnvironment : public ::testing::Environment {
5970
public:
6071
void SetUp() override {
@@ -68,12 +79,10 @@ class RegistrationTestEnvironment : public ::testing::Environment {
6879

6980
auto* lib_env = ::testing::AddGlobalTestEnvironment(new RegistrationTestEnvironment);
7081

71-
TEST(S3Test, FromUri) {
72-
ASSERT_OK_AND_ASSIGN(auto minio, GetMinioEnv()->GetOneServer());
73-
82+
TEST_F(S3ModuleTest, FromUri) {
7483
std::string path;
75-
ASSERT_OK_AND_ASSIGN(auto fs, FileSystemFromUri("s3://" + minio->access_key() + ":" +
76-
minio->secret_key() +
84+
ASSERT_OK_AND_ASSIGN(auto fs, FileSystemFromUri("s3://" + minio_->access_key() + ":" +
85+
minio_->secret_key() +
7786
"@bucket/somedir/subdir/subfile",
7887
&path));
7988

@@ -83,12 +92,11 @@ TEST(S3Test, FromUri) {
8392
"&allow_bucket_creation=0&allow_bucket_deletion=0");
8493
}
8594

86-
TEST(S3Test, FromUriAndOptionsCredentials) {
87-
ASSERT_OK_AND_ASSIGN(auto minio, GetMinioEnv()->GetOneServer());
95+
TEST_F(S3ModuleTest, FromUriAndOptionsCredentials) {
8896
std::string path;
8997
FileSystemFactoryOptions options{
90-
{"access_key", std::string(minio->access_key())},
91-
{"secret_key", std::string(minio->secret_key())},
98+
{"access_key", std::string(minio_->access_key())},
99+
{"secret_key", std::string(minio_->secret_key())},
92100
};
93101
// Credentials supplied via options, NOT in the URI.
94102
ASSERT_OK_AND_ASSIGN(
@@ -111,11 +119,10 @@ class NoopRetryStrategy : public S3RetryStrategy {
111119
};
112120
} // namespace
113121

114-
TEST(S3Test, FromUriAndOptionsRetryStrategy) {
115-
ASSERT_OK_AND_ASSIGN(auto minio, GetMinioEnv()->GetOneServer());
122+
TEST_F(S3ModuleTest, FromUriAndOptionsRetryStrategy) {
116123
FileSystemFactoryOptions options{
117-
{"access_key", std::string(minio->access_key())},
118-
{"secret_key", std::string(minio->secret_key())},
124+
{"access_key", std::string(minio_->access_key())},
125+
{"secret_key", std::string(minio_->secret_key())},
119126
{"retry_strategy",
120127
std::shared_ptr<S3RetryStrategy>(std::make_shared<NoopRetryStrategy>())},
121128
};

cpp/src/arrow/filesystem/s3fs_test.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,9 @@ class S3TestMixin : public AwsTestMixin {
192192
public:
193193
void SetUp() override {
194194
AwsTestMixin::SetUp();
195+
if (!GetMinioEnv(enable_tls_)->IsAvailable()) {
196+
GTEST_SKIP() << "Minio executable not found, skipping tests";
197+
}
195198

196199
// Starting the server may fail, for example if the generated port number
197200
// was "stolen" by another process. Run a dummy S3 operation to make sure it
@@ -624,6 +627,7 @@ class TestS3FS : public S3TestMixin {
624627
public:
625628
void SetUp() override {
626629
S3TestMixin::SetUp();
630+
if (IsSkipped()) return;
627631
// Most tests will create buckets
628632
options_.allow_bucket_creation = true;
629633
options_.allow_bucket_deletion = true;
@@ -1777,6 +1781,7 @@ class TestS3FSGeneric : public S3TestMixin, public GenericFileSystemTest {
17771781
public:
17781782
void SetUp() override {
17791783
S3TestMixin::SetUp();
1784+
if (IsSkipped()) return;
17801785
// Set up test bucket
17811786
{
17821787
Aws::S3::Model::CreateBucketRequest req;

0 commit comments

Comments
 (0)