Skip to content

Commit f551c00

Browse files
committed
test(compaction): fix flaky offline_compaction_test by confining log dir to a per-suite root
- Move the test log directory from /tmp/offline_compaction_test to /tmp/offline_compaction_test/log_dir so that the .work_XXXXXX / .backup_XXXXXX siblings created by tglogutil (make_tmp_dir_next_to) stay inside the per-suite root instead of /tmp itself. - This keeps find_sibling_dirs() from scanning /tmp, where temporary files of unrelated processes can vanish mid-scan and make directory iteration throw (the source of the nondeterministic "unknown file: Failure" crashes). - Recreate the per-suite root in SetUp() and remove it in TearDown(), so stale work/backup directories from an aborted run can never leak into a later test. - Drop the per-test leading cleanup calls that the per-suite wipe made redundant; keep the trailing ones that remove backups created by the test itself.
1 parent fd96fbd commit f551c00

1 file changed

Lines changed: 24 additions & 15 deletions

File tree

test/limestone/compaction/offline_compaction_test.cpp

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,30 @@ using namespace limestone::internal;
4444
// (datastore -> shutdown -> offline compaction -> restart) can be exercised.
4545
class offline_compaction_test : public compaction_test {
4646
public:
47-
// Use a dedicated directory so this suite does not collide with compaction_test,
48-
// which shares the same fixture base; ctest runs the two suites in parallel.
49-
offline_compaction_test() : compaction_test("/tmp/offline_compaction_test") {}
47+
// Place the log directory under a per-suite directory (rather than directly under
48+
// /tmp) so that this suite does not collide with compaction_test, which shares the
49+
// same fixture base and runs in parallel under ctest. Offline compaction creates its
50+
// work/backup directories next to the log directory (see make_tmp_dir_next_to), so
51+
// keeping the log directory one level down confines those siblings to test_root_ and
52+
// keeps find_sibling_dirs from scanning /tmp itself, where unrelated processes churn
53+
// temporary files.
54+
static constexpr char const* test_root_ = "/tmp/offline_compaction_test";
55+
offline_compaction_test() : compaction_test("/tmp/offline_compaction_test/log_dir") {}
56+
57+
// Recreate the per-suite root before each test and remove it afterwards. Wiping the
58+
// whole root also removes any .work_XXXXXX / .backup_XXXXXX siblings, including ones
59+
// left behind by a test that aborted before its own cleanup, so a later test can never
60+
// observe a stale working or backup directory.
61+
void SetUp() override {
62+
boost::filesystem::remove_all(test_root_);
63+
boost::filesystem::create_directories(test_root_);
64+
compaction_test::SetUp();
65+
}
66+
67+
void TearDown() override {
68+
compaction_test::TearDown();
69+
boost::filesystem::remove_all(test_root_);
70+
}
5071

5172
// Path to the offline compaction utility (tglogutil), relative to the test
5273
// executable's working directory. This matches the convention used by
@@ -482,9 +503,6 @@ TEST_F(offline_compaction_test, offline_compaction_preserves_manifest_file) {
482503
// whole directory (including the blob directory), not just the transaction log
483504
// files, so a regression that starts touching from_dir during a dry run is caught.
484505
TEST_F(offline_compaction_test, offline_compaction_dry_run_leaves_log_directory_intact) {
485-
remove_backup_dirs(); // clean up leftovers from an earlier aborted run
486-
remove_work_dirs();
487-
488506
gen_datastore();
489507
datastore_->switch_epoch(1);
490508
lc0_->begin_session();
@@ -556,8 +574,6 @@ TEST_F(offline_compaction_test, offline_compaction_ignores_epoch_option) {
556574
// With --make_backup, the blob directory must be copied (not moved) so that both the
557575
// compacted log directory and the backup directory keep the blob data.
558576
TEST_F(offline_compaction_test, offline_compaction_with_backup_copies_blob_directory) {
559-
remove_backup_dirs(); // clean up leftovers from an earlier aborted run
560-
561577
gen_datastore();
562578
datastore_->switch_epoch(1);
563579
lc0_->begin_session();
@@ -588,8 +604,6 @@ TEST_F(offline_compaction_test, offline_compaction_with_backup_copies_blob_direc
588604
// reported on stdout ("backup-directory: <path>") for the user to locate it. The
589605
// reported path must match the backup directory actually created.
590606
TEST_F(offline_compaction_test, offline_compaction_reports_backup_directory) {
591-
remove_backup_dirs(); // clean up leftovers from an earlier aborted run
592-
593607
gen_datastore();
594608
datastore_->switch_epoch(1);
595609
lc0_->begin_session();
@@ -660,8 +674,6 @@ TEST_F(offline_compaction_test, offline_compaction_does_not_drop_unexpected_file
660674
// as it was right before compaction: same set of files, same contents. This ensures the
661675
// backup is a faithful, fully recoverable image of the pre-compaction state.
662676
TEST_F(offline_compaction_test, offline_compaction_backup_matches_pre_compaction_state) {
663-
remove_backup_dirs(); // clean up leftovers from an earlier aborted run
664-
665677
gen_datastore();
666678
datastore_->switch_epoch(1);
667679
lc0_->begin_session();
@@ -707,9 +719,6 @@ TEST_F(offline_compaction_test, offline_compaction_with_backup_fails_when_workin
707719
// follows it and fails with ENOENT regardless of the caller's privileges, so this exercises
708720
// the copy-failure path even when the test runs as root (as it does in CI).
709721
TEST_F(offline_compaction_test, offline_compaction_backup_fails_when_blob_directory_copy_fails) {
710-
remove_backup_dirs();
711-
remove_work_dirs();
712-
713722
gen_datastore();
714723
datastore_->switch_epoch(1);
715724
lc0_->begin_session();

0 commit comments

Comments
 (0)