search

apple / foundationdb

FoundationDB - the open source, distributed, transactional key-value store
https://apple.github.io/foundationdb/
Apache License 2.0
14.37k stars 1.3k forks source link

Missing unit tests should silently succeed if FDB is not compiled with RocksDB support #6201

Open sfc-gh-jfu opened 2 years ago

sfc-gh-jfu commented 2 years ago

https://github.com/apple/foundationdb/pull/6200 exposed an issue with RocksDB unit tests not being found and breaking Joshua test suites if the flag to compile with RocksDB support is not enabled. https://github.com/apple/foundationdb/pull/4983 is the introduction to support for running non-simulation tests on Joshua, for reference.

sfc-gh-jfu commented 2 years ago

Confusingly, the SSD_ROCKSDB_EXPERIMENTAL flag in the CMakeLists does not correspond with the SSD_ROCKSDB_EXPERIMENTAL seen in the .cpp files as preprocessing directives. Shown here: https://github.com/apple/foundationdb/blob/master/cmake/FDBComponents.cmake#L171-L178 And here: https://github.com/apple/foundationdb/blob/master/fdbserver/CMakeLists.txt#L289-L299 This means that SSD_ROCKSDB_EXPERIMENTAL can be true for the CMakeLists but false when functioning as include guards such as these ones: https://github.com/apple/foundationdb/blob/master/fdbserver/KeyValueStoreRocksDB.actor.cpp#L28

It seems like the flag WITH_ROCKSDB_EXPERIMENTAL should be be the source of truth instead of SSD_ROCKSDB_EXPERIMENTAL in the CMake files, but that is not very intuitive or well-documented. Some type of refactor/renaming would be nice to see here.