From dbaa1828b59a5542c808770935b144aa98091980 Mon Sep 17 00:00:00 2001 From: Devon White Date: Wed, 24 Nov 2021 15:30:12 -0600 Subject: [PATCH] Address failures in DatabaseDownloader unittest --- src/ripple/net/HTTPDownloader.h | 8 ++- src/ripple/net/impl/HTTPDownloader.cpp | 14 +++++ src/test/net/DatabaseDownloader_test.cpp | 65 ++++++++++++++++++------ 3 files changed, 70 insertions(+), 17 deletions(-) diff --git a/src/ripple/net/HTTPDownloader.h b/src/ripple/net/HTTPDownloader.h index 1f2243a4fe4..39b9a904aa3 100644 --- a/src/ripple/net/HTTPDownloader.h +++ b/src/ripple/net/HTTPDownloader.h @@ -61,6 +61,12 @@ class HTTPDownloader : public std::enable_shared_from_this virtual ~HTTPDownloader() = default; + bool + sessionIsActive() const; + + bool + isStopping() const; + protected: // must be accessed through a shared_ptr // use make_XXX functions to create @@ -88,7 +94,7 @@ class HTTPDownloader : public std::enable_shared_from_this std::atomic stop_; // Used to protect sessionActive_ - std::mutex m_; + mutable std::mutex m_; bool sessionActive_; std::condition_variable c_; diff --git a/src/ripple/net/impl/HTTPDownloader.cpp b/src/ripple/net/impl/HTTPDownloader.cpp index 5ed2ceae0db..44d27466224 100644 --- a/src/ripple/net/impl/HTTPDownloader.cpp +++ b/src/ripple/net/impl/HTTPDownloader.cpp @@ -293,6 +293,20 @@ HTTPDownloader::stop() } } +bool +HTTPDownloader::sessionIsActive() const +{ + std::lock_guard lock(m_); + return sessionActive_; +} + +bool +HTTPDownloader::isStopping() const +{ + std::lock_guard lock(m_); + return stop_; +} + void HTTPDownloader::fail( boost::filesystem::path dstPath, diff --git a/src/test/net/DatabaseDownloader_test.cpp b/src/test/net/DatabaseDownloader_test.cpp index 749000dc50a..d4ed2ebcedf 100644 --- a/src/test/net/DatabaseDownloader_test.cpp +++ b/src/test/net/DatabaseDownloader_test.cpp @@ -19,7 +19,6 @@ #include #include -#include #include #include #include @@ -29,6 +28,8 @@ namespace ripple { namespace test { +#define REPORT_FAILURE(D) reportFailure(D, __FILE__, __LINE__) + class DatabaseDownloader_test : public beast::unit_test::suite { std::shared_ptr @@ -65,13 +66,10 @@ class DatabaseDownloader_test : public beast::unit_test::suite waitComplete() { std::unique_lock lk(m); - using namespace std::chrono_literals; -#if BOOST_OS_WINDOWS - auto constexpr timeout = 4s; -#else - auto constexpr timeout = 2s; -#endif - auto stat = cv.wait_for(lk, timeout, [this] { return called; }); + + auto stat = cv.wait_for( + lk, std::chrono::seconds(10), [this] { return called; }); + called = false; return stat; }; @@ -103,8 +101,29 @@ class DatabaseDownloader_test : public beast::unit_test::suite { return ptr_.get(); } + + DatabaseDownloader const* + operator->() const + { + return ptr_.get(); + } }; + void + reportFailure(Downloader const& dl, char const* file, int line) + { + std::stringstream ss; + ss << "Failed. LOGS:\n" + << dl.sink_.messages().str() + << "\nDownloadCompleter failure." + "\nDatabaseDownloader session active? " + << std::boolalpha << dl->sessionIsActive() + << "\nDatabaseDownloader is stopping? " << std::boolalpha + << dl->isStopping(); + + fail(ss.str(), file, line); + } + void testDownload(bool verify) { @@ -122,7 +141,7 @@ class DatabaseDownloader_test : public beast::unit_test::suite return cfg; })}; - Downloader downloader{env}; + Downloader dl{env}; // create a TrustedPublisherServer as a simple HTTP // server to request from. Use the /textfile endpoint @@ -133,7 +152,7 @@ class DatabaseDownloader_test : public beast::unit_test::suite *this, "downloads", "data", "", false, false}; // initiate the download and wait for the callback // to be invoked - auto stat = downloader->download( + auto stat = dl->download( server->local_endpoint().address().to_string(), std::to_string(server->local_endpoint().port()), "/textfile", @@ -142,12 +161,12 @@ class DatabaseDownloader_test : public beast::unit_test::suite std::function{std::ref(cb)}); if (!BEAST_EXPECT(stat)) { - log << "Failed. LOGS:\n" + downloader.sink_.messages().str(); + REPORT_FAILURE(dl); return; } if (!BEAST_EXPECT(cb.waitComplete())) { - log << "Failed. LOGS:\n" + downloader.sink_.messages().str(); + REPORT_FAILURE(dl); return; } BEAST_EXPECT(cb.dest == data.file()); @@ -187,7 +206,10 @@ class DatabaseDownloader_test : public beast::unit_test::suite datafile.file(), std::function{ std::ref(cb)})); - BEAST_EXPECT(cb.waitComplete()); + if (!BEAST_EXPECT(cb.waitComplete())) + { + REPORT_FAILURE(dl); + } BEAST_EXPECT(!boost::filesystem::exists(datafile.file())); BEAST_EXPECTS( dl.sink_.messages().str().find("async_resolve") != @@ -211,7 +233,10 @@ class DatabaseDownloader_test : public beast::unit_test::suite 11, datafile.file(), std::function{std::ref(cb)})); - BEAST_EXPECT(cb.waitComplete()); + if (!BEAST_EXPECT(cb.waitComplete())) + { + REPORT_FAILURE(dl); + } BEAST_EXPECT(!boost::filesystem::exists(datafile.file())); BEAST_EXPECTS( dl.sink_.messages().str().find("async_connect") != @@ -231,7 +256,10 @@ class DatabaseDownloader_test : public beast::unit_test::suite 11, datafile.file(), std::function{std::ref(cb)})); - BEAST_EXPECT(cb.waitComplete()); + if (!BEAST_EXPECT(cb.waitComplete())) + { + REPORT_FAILURE(dl); + } BEAST_EXPECT(!boost::filesystem::exists(datafile.file())); BEAST_EXPECTS( dl.sink_.messages().str().find("async_handshake") != @@ -251,7 +279,10 @@ class DatabaseDownloader_test : public beast::unit_test::suite 11, datafile.file(), std::function{std::ref(cb)})); - BEAST_EXPECT(cb.waitComplete()); + if (!BEAST_EXPECT(cb.waitComplete())) + { + REPORT_FAILURE(dl); + } BEAST_EXPECT(!boost::filesystem::exists(datafile.file())); BEAST_EXPECTS( dl.sink_.messages().str().find("Insufficient disk space") != @@ -270,6 +301,8 @@ class DatabaseDownloader_test : public beast::unit_test::suite } }; +#undef REPORT_FAILURE + BEAST_DEFINE_TESTSUITE(DatabaseDownloader, net, ripple); } // namespace test } // namespace ripple