From 988d8ff9ace1018f2ba5fbedb7c5532ed392658a Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 16 Aug 2022 17:47:02 -0500 Subject: [PATCH] test: possibly fix rename-test flakes (#3653) * test: possibly fix rename-test flakes * ci: add gettext-dev to alpine workflow * ci: make utils if tests change, since transmission-show is needed * ci: add linux-headers to alpine workflow --- .github/workflows/actions.yml | 9 ++--- tests/libtransmission/rename-test.cc | 52 +++++++++++++-------------- tests/libtransmission/test-fixtures.h | 8 +---- 3 files changed, 32 insertions(+), 37 deletions(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index ee097a1a9..23390bf67 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -20,7 +20,7 @@ jobs: make-qt: ${{ steps.check-main-push.outputs.is-main-push == '1' || steps.check-diffs.outputs.qt-changed == '1' }} make-source-tarball: ${{ steps.check-main-push.outputs.is-main-push == '1' || steps.check-diffs.outputs.any-code-changed == '1' }} make-tests: ${{ steps.check-main-push.outputs.is-main-push == '1' || steps.check-diffs.outputs.tests-changed == '1' }} - make-utils: ${{ steps.check-main-push.outputs.is-main-push == '1' || steps.check-diffs.outputs.utils-changed == '1' }} + make-utils: ${{ steps.check-main-push.outputs.is-main-push == '1' || steps.check-diffs.outputs.utils-changed == '1' || steps.check-diffs.outputs.tests-changed == '1' }} make-web: ${{ steps.check-main-push.outputs.is-main-push == '1' || steps.check-diffs.outputs.web-changed == '1' }} test-style: ${{ steps.check-main-push.outputs.is-main-push == '1' || steps.check-diffs.outputs.our-code-changed == '1' }} steps: @@ -243,13 +243,14 @@ jobs: apk add \ ca-certificates \ cmake \ - g++ \ - gettext \ - git \ curl-dev \ fmt-dev \ + g++ \ + gettext-dev \ + git \ libevent-dev \ libpsl \ + linux-headers \ miniupnpc-dev \ ninja \ pkgconfig \ diff --git a/tests/libtransmission/rename-test.cc b/tests/libtransmission/rename-test.cc index abf2601af..0530b4a3d 100644 --- a/tests/libtransmission/rename-test.cc +++ b/tests/libtransmission/rename-test.cc @@ -230,17 +230,17 @@ TEST_F(RenameTest, multifileTorrent) { char* str; auto constexpr TotalSize = size_t{ 67 }; - auto const expected_files = std::array{ - "Felidae/Felinae/Acinonyx/Cheetah/Chester", - "Felidae/Felinae/Felis/catus/Kyphi", - "Felidae/Felinae/Felis/catus/Saffron", - "Felidae/Pantherinae/Panthera/Tiger/Tony", + auto constexpr ExpectedFiles = std::array{ + "Felidae/Felinae/Acinonyx/Cheetah/Chester"sv, + "Felidae/Felinae/Felis/catus/Kyphi"sv, + "Felidae/Felinae/Felis/catus/Saffron"sv, + "Felidae/Pantherinae/Panthera/Tiger/Tony"sv, }; - auto const expected_contents = std::array{ - "It ain't easy bein' cheesy.\n", - "Inquisitive\n", - "Tough\n", - "They’re Grrrrreat!\n", + auto constexpr ExpectedContents = std::array{ + "It ain't easy bein' cheesy.\n"sv, + "Inquisitive\n"sv, + "Tough\n"sv, + "They’re Grrrrreat!\n"sv, }; auto* ctor = tr_ctorNew(session_); @@ -263,7 +263,7 @@ TEST_F(RenameTest, multifileTorrent) for (tr_file_index_t i = 0; i < 4; ++i) { - EXPECT_EQ(expected_files[i], tr_torrentFile(tor, i).name); + EXPECT_EQ(ExpectedFiles[i], tr_torrentFile(tor, i).name); } // sanity check the (empty) stats @@ -300,12 +300,12 @@ TEST_F(RenameTest, multifileTorrent) // rename a branch... EXPECT_EQ(0, torrentRenameAndWait(tor, "Felidae/Felinae/Felis/catus", "placeholder")); - EXPECT_EQ(expected_files[0], tr_torrentFile(tor, 0).name); + EXPECT_EQ(ExpectedFiles[0], tr_torrentFile(tor, 0).name); EXPECT_STREQ("Felidae/Felinae/Felis/placeholder/Kyphi", tr_torrentFile(tor, 1).name); EXPECT_STREQ("Felidae/Felinae/Felis/placeholder/Saffron", tr_torrentFile(tor, 2).name); - EXPECT_EQ(expected_files[3], tr_torrentFile(tor, 3).name); - EXPECT_TRUE(testFileExistsAndConsistsOfThisString(tor, 1, expected_contents[1])); - EXPECT_TRUE(testFileExistsAndConsistsOfThisString(tor, 2, expected_contents[2])); + EXPECT_EQ(ExpectedFiles[3], tr_torrentFile(tor, 3).name); + EXPECT_TRUE(testFileExistsAndConsistsOfThisString(tor, 1, ExpectedContents[1])); + EXPECT_TRUE(testFileExistsAndConsistsOfThisString(tor, 2, ExpectedContents[2])); // (while the branch is renamed: confirm that the .resume file remembers the changes) tr_resume::save(tor); @@ -313,18 +313,18 @@ TEST_F(RenameTest, multifileTorrent) tor->setFileSubpath(1, "gabba gabba hey"sv); auto const loaded = tr_resume::load(tor, tr_resume::All, ctor, nullptr); EXPECT_NE(decltype(loaded){ 0 }, (loaded & tr_resume::Filenames)); - EXPECT_EQ(expected_files[0], tr_torrentFile(tor, 0).name); + EXPECT_EQ(ExpectedFiles[0], tr_torrentFile(tor, 0).name); EXPECT_STREQ("Felidae/Felinae/Felis/placeholder/Kyphi", tr_torrentFile(tor, 1).name); EXPECT_STREQ("Felidae/Felinae/Felis/placeholder/Saffron", tr_torrentFile(tor, 2).name); - EXPECT_EQ(expected_files[3], tr_torrentFile(tor, 3).name); + EXPECT_EQ(ExpectedFiles[3], tr_torrentFile(tor, 3).name); // ...and back again EXPECT_EQ(0, torrentRenameAndWait(tor, "Felidae/Felinae/Felis/placeholder", "catus")); for (tr_file_index_t i = 0; i < 4; ++i) { - EXPECT_EQ(expected_files[i], tr_torrentFile(tor, i).name); - EXPECT_TRUE(testFileExistsAndConsistsOfThisString(tor, i, expected_contents[i])); + EXPECT_EQ(ExpectedFiles[i], tr_torrentFile(tor, i).name); + EXPECT_TRUE(testFileExistsAndConsistsOfThisString(tor, i, ExpectedContents[i])); } /*** @@ -343,7 +343,7 @@ TEST_F(RenameTest, multifileTorrent) tr_free(str); sync(); blockingTorrentVerify(tor); - testFileExistsAndConsistsOfThisString(tor, 0, expected_contents[0]); + testFileExistsAndConsistsOfThisString(tor, 0, ExpectedContents[0]); for (tr_file_index_t i = 1; i <= 2; ++i) { @@ -352,21 +352,21 @@ TEST_F(RenameTest, multifileTorrent) tr_free(str); } - testFileExistsAndConsistsOfThisString(tor, 3, expected_contents[3]); + testFileExistsAndConsistsOfThisString(tor, 3, ExpectedContents[3]); // rename a branch... EXPECT_EQ(0, torrentRenameAndWait(tor, "Felidae/Felinae/Felis/catus", "foo")); - EXPECT_EQ(expected_files[0], tr_torrentFile(tor, 0).name); + EXPECT_EQ(ExpectedFiles[0], tr_torrentFile(tor, 0).name); EXPECT_STREQ("Felidae/Felinae/Felis/foo/Kyphi", tr_torrentFile(tor, 1).name); EXPECT_STREQ("Felidae/Felinae/Felis/foo/Saffron", tr_torrentFile(tor, 2).name); - EXPECT_EQ(expected_files[3], tr_torrentFile(tor, 3).name); + EXPECT_EQ(ExpectedFiles[3], tr_torrentFile(tor, 3).name); // ...and back again EXPECT_EQ(0, torrentRenameAndWait(tor, "Felidae/Felinae/Felis/foo", "catus")); for (tr_file_index_t i = 0; i < 4; ++i) { - EXPECT_EQ(expected_files[i], tr_torrentFile(tor, i).name); + EXPECT_EQ(ExpectedFiles[i], tr_torrentFile(tor, i).name); } EXPECT_EQ(0, torrentRenameAndWait(tor, "Felidae", "gabba")); @@ -379,7 +379,7 @@ TEST_F(RenameTest, multifileTorrent) for (tr_file_index_t i = 0; i < 4; ++i) { EXPECT_STREQ(strings[i], tr_torrentFile(tor, i).name); - testFileExistsAndConsistsOfThisString(tor, i, expected_contents[i]); + testFileExistsAndConsistsOfThisString(tor, i, ExpectedContents[i]); } // rename the root, then a branch, and then a leaf... @@ -394,7 +394,7 @@ TEST_F(RenameTest, multifileTorrent) for (tr_file_index_t i = 0; i < 4; ++i) { EXPECT_STREQ(strings[i], tr_torrentFile(tor, i).name); - testFileExistsAndConsistsOfThisString(tor, i, expected_contents[i]); + testFileExistsAndConsistsOfThisString(tor, i, ExpectedContents[i]); } tr_ctorFree(ctor); diff --git a/tests/libtransmission/test-fixtures.h b/tests/libtransmission/test-fixtures.h index e312b5325..638ac093d 100644 --- a/tests/libtransmission/test-fixtures.h +++ b/tests/libtransmission/test-fixtures.h @@ -459,13 +459,7 @@ protected: { EXPECT_NE(nullptr, tor->session); tr_wait_msec(100); - EXPECT_TRUE(waitFor( - [tor]() - { - auto const activity = tr_torrentGetActivity(tor); - return activity != TR_STATUS_CHECK && activity != TR_STATUS_CHECK_WAIT && tor->checked_pieces_.hasAll(); - }, - 4000)); + EXPECT_TRUE(waitFor([tor]() { return tor->verifyState() == TR_VERIFY_NONE && tor->checked_pieces_.hasAll(); }, 4000)); } void blockingTorrentVerify(tr_torrent* tor) const