Skip to content

Commit 385f691

Browse files
committed
refactor(test): declutter some tests; remove extra asserts
A code review with https://github.com/CodeItQuick revealed that deleting_parent_of_missing_file_is_not_detected_as_a_change is asserting too much. In particular, asserting that watch_and_load_for_file succeeds is unnecessary noise in the test. Remove excessive asserts from tests. Also, add a test which has the same setup as deleting_parent_of_missing_file_is_not_detected_as_a_change but is more deliberate in asserting the returned config.
1 parent 4c15458 commit 385f691

File tree

1 file changed

+17
-11
lines changed

1 file changed

+17
-11
lines changed

test/test-configuration-loader.cpp

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,19 @@ TEST_F(Test_Configuration_Loader,
620620
EXPECT_SAME_FILE(*(*loaded_config)->config_path, config_file);
621621
}
622622

623+
TEST_F(Test_Configuration_Loader,
624+
finding_config_succeeds_even_if_file_and_config_file_are_missing) {
625+
std::string temp_dir = this->make_temporary_directory();
626+
std::string parent_dir = temp_dir + "/dir";
627+
create_directory_or_exit(parent_dir);
628+
629+
std::string js_file = parent_dir + "/hello.js";
630+
Configuration_Loader loader(Basic_Configuration_Filesystem::instance());
631+
auto loaded_config = loader.load_for_file(js_file);
632+
EXPECT_TRUE(loaded_config.ok()) << loaded_config.error_to_string();
633+
EXPECT_EQ(*loaded_config, nullptr);
634+
}
635+
623636
TEST_F(Test_Configuration_Loader,
624637
finding_config_succeeds_even_if_directory_is_missing) {
625638
std::string temp_dir = this->make_temporary_directory();
@@ -641,8 +654,7 @@ TEST_F(Test_Configuration_Loader,
641654

642655
std::string js_file = parent_dir + "/hello.js";
643656
Change_Detecting_Configuration_Loader loader;
644-
auto loaded_config = loader.watch_and_load_for_file(js_file, &js_file);
645-
EXPECT_TRUE(loaded_config.ok()) << loaded_config.error_to_string();
657+
loader.watch_and_load_for_file(js_file, &js_file);
646658

647659
EXPECT_EQ(::rmdir(parent_dir.c_str()), 0)
648660
<< "failed to delete " << parent_dir << ": " << std::strerror(errno);
@@ -1302,9 +1314,7 @@ TEST_F(Test_Configuration_Loader,
13021314
write_file_or_exit(config_file, u8R"({"globals": {"before": true}})"_sv);
13031315

13041316
Change_Detecting_Configuration_Loader loader;
1305-
auto loaded_config =
1306-
loader.watch_and_load_for_file(js_file, /*token=*/nullptr);
1307-
ASSERT_TRUE(loaded_config.ok()) << loaded_config.error_to_string();
1317+
loader.watch_and_load_for_file(js_file, /*token=*/nullptr);
13081318

13091319
write_file_or_exit(config_file, u8R"({"globals": {"during": true}})"_sv);
13101320
loader.unwatch_file(js_file);
@@ -1340,12 +1350,8 @@ TEST_F(Test_Configuration_Loader,
13401350
write_file_or_exit(config_file, u8R"({"globals": {"before": true}})"_sv);
13411351

13421352
Change_Detecting_Configuration_Loader loader;
1343-
auto loaded_config_1 =
1344-
loader.watch_and_load_for_file(js_file_1, /*token=*/nullptr);
1345-
ASSERT_TRUE(loaded_config_1.ok()) << loaded_config_1.error_to_string();
1346-
auto loaded_config_2 =
1347-
loader.watch_and_load_for_file(js_file_2, /*token=*/nullptr);
1348-
ASSERT_TRUE(loaded_config_1.ok()) << loaded_config_2.error_to_string();
1353+
loader.watch_and_load_for_file(js_file_1, /*token=*/nullptr);
1354+
loader.watch_and_load_for_file(js_file_2, /*token=*/nullptr);
13491355

13501356
write_file_or_exit(config_file, u8R"({"globals": {"during": true}})"_sv);
13511357
loader.unwatch_all_files();

0 commit comments

Comments
 (0)