From 60d1f782395868a2ea8d43a263194b05329ea08b Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 23 Oct 2025 16:44:52 +0300 Subject: [PATCH 1/6] database_test: Introduce collect_files() helper It returns a set of files in a given directoy. Will be used by all next patches. Implemented using directory_lister, not lister::scan_dir in order to help removing the latter one in the future. Signed-off-by: Pavel Emelyanov --- test/boost/database_test.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/boost/database_test.cc b/test/boost/database_test.cc index 31572a3b7e..1ad343a08a 100644 --- a/test/boost/database_test.cc +++ b/test/boost/database_test.cc @@ -623,6 +623,15 @@ future<> take_snapshot(cql_test_env& e, sstring ks_name = "ks", sstring cf_name } } +future> collect_files(fs::path path) { + std::set ret; + directory_lister lister(path, lister::dir_entry_types::of()); + while (auto de = co_await lister.get()) { + ret.insert(de->name); + } + co_return ret; +} + static future<> snapshot_works(const std::string& table_name) { return do_with_some_data({"cf"}, [table_name] (cql_test_env& e) { take_snapshot(e, "ks", table_name).get(); From e1f326d133d175a119450cee6a4ff469da2397d7 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 23 Oct 2025 17:01:54 +0300 Subject: [PATCH 2/6] database_test: Use collectz_files() to count files in directory Some test cases want to see that there are more than one file in a directory, so they can just re-use the new helper. Much shorter this way. Signed-off-by: Pavel Emelyanov --- test/boost/database_test.cc | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/test/boost/database_test.cc b/test/boost/database_test.cc index 1ad343a08a..99a80782df 100644 --- a/test/boost/database_test.cc +++ b/test/boost/database_test.cc @@ -785,11 +785,7 @@ SEASTAR_TEST_CASE(clear_snapshot) { take_snapshot(e).get(); auto& cf = e.local_db().find_column_family("ks", "cf"); - unsigned count = 0; - lister::scan_dir((table_dir(cf) / sstables::snapshots_dir / "test"), lister::dir_entry_types::of(), [&count] (fs::path parent_dir, directory_entry de) { - count++; - return make_ready_future<>(); - }).get(); + unsigned count = collect_files(table_dir(cf) / sstables::snapshots_dir / "test").get().size(); BOOST_REQUIRE_GT(count, 1); // expect more than the manifest alone e.local_db().clear_snapshot("test", {"ks"}, "").get(); @@ -820,12 +816,8 @@ SEASTAR_TEST_CASE(clear_multiple_snapshots) { } for (auto i = 0; i < num_snapshots; i++) { - unsigned count = 0; testlog.debug("Verifying {}", snapshots_dir / snapshot_name(i)); - lister::scan_dir(snapshots_dir / snapshot_name(i), lister::dir_entry_types::of(), [&count] (fs::path parent_dir, directory_entry de) { - count++; - return make_ready_future<>(); - }).get(); + unsigned count = collect_files(snapshots_dir / snapshot_name(i)).get().size(); BOOST_REQUIRE_GT(count, 1); // expect more than the manifest alone } From 365044cdbb47717c4defe56646ca9421d503e4ff Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 23 Oct 2025 17:04:13 +0300 Subject: [PATCH 3/6] database_test: Use collect_files() to remove files Some test cases remove files from table directory to perform some checks over the taken snapshots. Using collect_files() helper makes the code easier to read. Signed-off-by: Pavel Emelyanov --- test/boost/database_test.cc | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/boost/database_test.cc b/test/boost/database_test.cc index 99a80782df..7e5da58f1d 100644 --- a/test/boost/database_test.cc +++ b/test/boost/database_test.cc @@ -715,10 +715,10 @@ SEASTAR_TEST_CASE(snapshot_list_okay) { BOOST_REQUIRE_EQUAL(sd.live, 0); BOOST_REQUIRE_GT(sd.total, 0); - lister::scan_dir(table_dir(cf), lister::dir_entry_types::of(), [] (fs::path parent_dir, directory_entry de) { - fs::remove(parent_dir / de.name); - return make_ready_future<>(); - }).get(); + auto table_directory = table_dir(cf); + for (auto& f : collect_files(table_directory).get()) { + fs::remove(table_directory / f); + } auto sd_post_deletion = cf.get_snapshot_details().get().at("test"); @@ -900,10 +900,10 @@ SEASTAR_TEST_CASE(test_snapshot_ctl_details) { BOOST_REQUIRE_EQUAL(sc_sd.details.live, sd.live); BOOST_REQUIRE_EQUAL(sc_sd.details.total, sd.total); - lister::scan_dir(table_dir(cf), lister::dir_entry_types::of(), [] (fs::path parent_dir, directory_entry de) { - fs::remove(parent_dir / de.name); - return make_ready_future<>(); - }).get(); + auto table_directory = table_dir(cf); + for (auto& f : collect_files(table_directory).get()) { + fs::remove(table_directory / f); + } auto sd_post_deletion = cf.get_snapshot_details().get().at("test"); @@ -942,10 +942,10 @@ SEASTAR_TEST_CASE(test_snapshot_ctl_true_snapshots_size) { auto sc_live_size = sc.local().true_snapshots_size().get(); BOOST_REQUIRE_EQUAL(sc_live_size, sd.live); - lister::scan_dir(table_dir(cf), lister::dir_entry_types::of(), [] (fs::path parent_dir, directory_entry de) { - fs::remove(parent_dir / de.name); - return make_ready_future<>(); - }).get(); + auto table_directory = table_dir(cf); + for (auto& f : collect_files(table_directory).get()) { + fs::remove(table_directory / f); + } auto sd_post_deletion = cf.get_snapshot_details().get().at("test"); From 5a25d74b12cb28bdbe610f0e516744648f462fdc Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 23 Oct 2025 16:47:05 +0300 Subject: [PATCH 4/6] database_test: Simplify snapshot_works() tests No functional changes here, just make use of the new lister to shorten the code. A small side effect -- if the test fails because contents of directories changes, it will print the exact difference in logs, not just that N files are missing/present. Signed-off-by: Pavel Emelyanov --- test/boost/database_test.cc | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/test/boost/database_test.cc b/test/boost/database_test.cc index 7e5da58f1d..684f9239ac 100644 --- a/test/boost/database_test.cc +++ b/test/boost/database_test.cc @@ -636,29 +636,21 @@ static future<> snapshot_works(const std::string& table_name) { return do_with_some_data({"cf"}, [table_name] (cql_test_env& e) { take_snapshot(e, "ks", table_name).get(); - std::set expected = { - "manifest.json", - "schema.cql" - }; - auto& cf = e.local_db().find_column_family("ks", table_name); auto table_directory = table_dir(cf); auto snapshot_dir = table_directory / sstables::snapshots_dir / "test"; - lister::scan_dir(table_directory, lister::dir_entry_types::of(), [&expected](fs::path, directory_entry de) { - expected.insert(de.name); - return make_ready_future<>(); - }).get(); + auto in_table_dir = collect_files(table_directory).get(); // snapshot triggered a flush and wrote the data down. - BOOST_REQUIRE_GE(expected.size(), 11); + BOOST_REQUIRE_GE(in_table_dir.size(), 9); + auto in_snapshot_dir = collect_files(snapshot_dir).get(); + + in_table_dir.insert("manifest.json"); + in_table_dir.insert("schema.cql"); // all files were copied and manifest was generated - lister::scan_dir(snapshot_dir, lister::dir_entry_types::of(), [&expected](fs::path, directory_entry de) { - expected.erase(de.name); - return make_ready_future<>(); - }).get(); + BOOST_REQUIRE_EQUAL(in_table_dir, in_snapshot_dir); - BOOST_REQUIRE_EQUAL(expected.size(), 0); return make_ready_future<>(); }, true); } From c8492b35628ead7b3b2a5f5a319435aa335866a0 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 23 Oct 2025 16:47:59 +0300 Subject: [PATCH 5/6] database_test: Improve snapshot_skip_flush_works test It has two inaccuracies. First, when checking the contents of table directory, it uses pre-populated expected list with "manifest.json" in it. Weird. Second, when cechking the contents of snapshot directory it doesn't check if the "schema.cql" is there. It's always there, but if something breaks in the future it may come unnoticed. Signed-off-by: Pavel Emelyanov --- test/boost/database_test.cc | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/test/boost/database_test.cc b/test/boost/database_test.cc index 684f9239ac..1e6d0bb373 100644 --- a/test/boost/database_test.cc +++ b/test/boost/database_test.cc @@ -671,26 +671,13 @@ SEASTAR_TEST_CASE(snapshot_skip_flush_works) { return do_with_some_data({"cf"}, [] (cql_test_env& e) { take_snapshot(e, "ks", "cf", "test", true /* skip_flush */).get(); - std::set expected = { - "manifest.json", - }; - auto& cf = e.local_db().find_column_family("ks", "cf"); - lister::scan_dir(table_dir(cf), lister::dir_entry_types::of(), [&expected] (fs::path parent_dir, directory_entry de) { - expected.insert(de.name); - return make_ready_future<>(); - }).get(); + + auto in_table_dir = collect_files(table_dir(cf)).get(); // Snapshot did not trigger a flush. - // Only "manifest.json" is expected. - BOOST_REQUIRE_EQUAL(expected.size(), 1); - - // all files were copied and manifest was generated - lister::scan_dir((table_dir(cf) / sstables::snapshots_dir / "test"), lister::dir_entry_types::of(), [&expected] (fs::path parent_dir, directory_entry de) { - expected.erase(de.name); - return make_ready_future<>(); - }).get(); - - BOOST_REQUIRE_EQUAL(expected.size(), 0); + BOOST_REQUIRE(in_table_dir.empty()); + auto in_snapshot_dir = collect_files(table_dir(cf) / sstables::snapshots_dir / "test").get(); + BOOST_REQUIRE_EQUAL(in_snapshot_dir, std::set({"manifest.json", "schema.cql"})); return make_ready_future<>(); }); } From 05d711f22165900ee68559b3742d70a8639d6842 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Thu, 23 Oct 2025 17:10:10 +0300 Subject: [PATCH 6/6] database_test: Simplify snapshot_with_quarantine_works() test The test collects Data files from table dir, then _all_ files from snapshot dir and then checks whether the former is the subset of the latter. Using std::includes over two sets makes the code much shorter. Signed-off-by: Pavel Emelyanov --- test/boost/database_test.cc | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/test/boost/database_test.cc b/test/boost/database_test.cc index 1e6d0bb373..97150151f2 100644 --- a/test/boost/database_test.cc +++ b/test/boost/database_test.cc @@ -1465,18 +1465,9 @@ SEASTAR_TEST_CASE(snapshot_with_quarantine_works) { auto& cf = db.local().find_column_family("ks", "cf"); + auto in_snap_dir = co_await collect_files(table_dir(cf) / sstables::snapshots_dir / "test"); // all files were copied and manifest was generated - co_await lister::scan_dir((table_dir(cf) / sstables::snapshots_dir / "test"), lister::dir_entry_types::of(), [&expected] (fs::path parent_dir, directory_entry de) { - testlog.debug("Found in snapshots: {}", de.name); - expected.erase(de.name); - return make_ready_future<>(); - }); - - if (!expected.empty()) { - testlog.error("Not in snapshots: {}", expected); - } - - BOOST_REQUIRE(expected.empty()); + BOOST_REQUIRE(std::includes(in_snap_dir.begin(), in_snap_dir.end(), expected.begin(), expected.end())); }); }