-
Notifications
You must be signed in to change notification settings - Fork 4.1k
[C++] Tests incorrectly skipped due to missing definitions #40327
Description
Describe the bug, including details regarding any error messages, version, and platform.
Some C++ tests are being incorrectly skipped due to missing preprocessor definitions.
Eg. If I build Arrow C++ with Parquet encryption and Snappy enabled, then run the Parquet encryption key management tests:
cd cpp
cmake -B build -S . \
--preset ninja-debug-python \
-DCMAKE_INSTALL_PREFIX=$PWD/../dist \
-DCMAKE_INSTALL_LIBDIR=lib \
-DARROW_BUILD_TESTS=ON \
-DARROW_BUILD_INTEGRATION=ON \
-DARROW_BUILD_STATIC="OFF" \
-DARROW_COMPUTE="ON" \
-DARROW_CSV="ON" \
-DARROW_ACERO="ON" \
-DARROW_DATASET="ON" \
-DARROW_FILESYSTEM="ON" \
-DARROW_JSON="ON" \
-DARROW_MIMALLOC="ON" \
-DARROW_ORC="ON" \
-DARROW_PARQUET="ON" \
-DARROW_SUBSTRAIT="ON" \
-DARROW_ENABLE_THREADING="ON" \
-DARROW_WITH_BROTLI="ON" \
-DARROW_WITH_BZ2="ON" \
-DARROW_WITH_LZ4="ON" \
-DARROW_WITH_RE2="ON" \
-DARROW_WITH_SNAPPY="ON" \
-DARROW_WITH_UTF8PROC="ON" \
-DARROW_WITH_ZLIB="ON" \
-DARROW_WITH_ZSTD="ON" \
-DPARQUET_REQUIRE_ENCRYPTION="ON"
cmake --build build
./build/debug/parquet-encryption-key-management-test
I see that a bunch of tests are skipped saying that Snappy hasn't been enabled:
...
[ RUN ] TestEncryptionKeyManagement.WrapLocally
/home/adam/dev/arrow/cpp/src/parquet/encryption/key_management_test.cc:57: Skipped
Test requires Snappy compression
[ SKIPPED ] TestEncryptionKeyManagement.WrapLocally (0 ms)
...
Due to this check:
arrow/cpp/src/parquet/encryption/key_management_test.cc
Lines 55 to 65 in 6406676
| void SetUp() { | |
| #ifndef ARROW_WITH_SNAPPY | |
| GTEST_SKIP() << "Test requires Snappy compression"; | |
| #endif | |
| key_list_ = BuildKeyMap(kColumnMasterKeyIds, kColumnMasterKeys, kFooterMasterKeyId, | |
| kFooterMasterKey); | |
| new_key_list_ = BuildKeyMap(kColumnMasterKeyIds, kNewColumnMasterKeys, | |
| kFooterMasterKeyId, kNewFooterMasterKey); | |
| column_key_mapping_ = BuildColumnKeyMapping(); | |
| temp_dir_ = temp_data_dir().ValueOrDie(); | |
| } |
If I comment out that check then the tests pass correctly.
I used git bisect and found that this was caused by #40222 (cc @kou).
I haven't looked for any other tests that are also incorrectly skipped so suspect there could be a lot more like this.
Also, if I run the tests at the previous commit before that PR merge (193e39c) I see some tests were already incorrectly skipped due to another missing definition for ARROW_ENABLE_THREADING. So even if the above PR is reverted or fixed, it looks like this was already a problem:
[ RUN ] TestEncryptionKeyManagementMultiThread.WrapLocally
/home/adam/dev/arrow/cpp/src/parquet/encryption/key_management_test.cc:328: Skipped
Test requires threading support
[ SKIPPED ] TestEncryptionKeyManagementMultiThread.WrapLocally (0 ms)
I tried to find if this was also a recent regression but these tests were also skipped back at commit ad7f6ef, and prior to that I'm running into issues building Arrow due to my version of gtest being too new.
I'm testing with GCC 13.2.1 and cmake 3.27.7 on Fedora 39, and initially noticed this at commit 6406676 on the main branch.
Component(s)
C++