Address failures in DatabaseDownloader unittest

manojsdoshi · May 10, 2022 · 767dd4f · 767dd4f
1 parent 01c37fe
commit 767dd4f
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 17 deletions.
diff --git a/src/ripple/net/HTTPDownloader.h b/src/ripple/net/HTTPDownloader.h
@@ -61,6 +61,12 @@ class HTTPDownloader : public std::enable_shared_from_this<HTTPDownloader>
 
  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<HTTPDownloader>
  std::atomic<bool> 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
@@ -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
@@ -19,7 +19,6 @@
 
 #include <ripple/net/DatabaseDownloader.h>
 #include <boost/filesystem/operations.hpp>
-#include <boost/predef.h>
 #include <condition_variable>
 #include <mutex>
 #include <test/jtx.h>
@@ -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<TrustedPublisherServer>
@@ -65,13 +66,10 @@ class DatabaseDownloader_test : public beast::unit_test::suite
  waitComplete()
  {
  std::unique_lock<std::mutex> 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<void(boost::filesystem::path)>{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<void(boost::filesystem::path)>{
  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<void(boost::filesystem::path)>{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<void(boost::filesystem::path)>{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<void(boost::filesystem::path)>{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