From 86a183a847aefc85a1c8d3b63cb31d177deefaec Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Tue, 18 Jul 2023 16:19:52 -0700 Subject: [PATCH] test: Use a random HTTP port for web server tests (#1248) This will pick a random HTTP port for the test web server, and retry up to 10 times if the chosen port number is in use. --- packager/file/http_file_unittest.cc | 35 ++++--------- .../media/base/http_key_fetcher_unittest.cc | 28 +++------- packager/media/test/test_web_server.cc | 51 ++++++++++++++----- packager/media/test/test_web_server.h | 23 ++++++++- 4 files changed, 77 insertions(+), 60 deletions(-) diff --git a/packager/file/http_file_unittest.cc b/packager/file/http_file_unittest.cc index e061215fd4..9c0030abc2 100644 --- a/packager/file/http_file_unittest.cc +++ b/packager/file/http_file_unittest.cc @@ -24,18 +24,6 @@ namespace shaka { namespace { -// A completely arbitrary port number, unlikely to be in use. -const int kTestServerPort = 58405; -// Reflects back the method, body, and headers of the request as JSON. -const std::string kTestServerReflect = "http://localhost:58405/reflect"; -// Returns the requested HTTP status code. -const std::string kTestServer404 = "http://localhost:58405/status?code=404"; -// Returns after the requested delay. -const std::string kTestServerLongDelay = - "http://localhost:58405/delay?seconds=8"; -const std::string kTestServerShortDelay = - "http://localhost:58405/delay?seconds=1"; - const std::vector kNoHeaders; const std::string kNoContentType; const std::string kBinaryContentType = "application/octet-stream"; @@ -90,16 +78,15 @@ nlohmann::json HandleResponse(const FilePtr& file) { // enough. class HttpFileTest : public testing::Test { protected: - void SetUp() override { ASSERT_TRUE(server_.Start(kTestServerPort)); } + void SetUp() override { ASSERT_TRUE(server_.Start()); } - private: media::TestWebServer server_; }; } // namespace TEST_F(HttpFileTest, BasicGet) { - FilePtr file(new HttpFile(HttpMethod::kGet, kTestServerReflect, + FilePtr file(new HttpFile(HttpMethod::kGet, server_.ReflectUrl(), kNoContentType, kNoHeaders, kDefaultTestTimeout)); ASSERT_TRUE(file); ASSERT_TRUE(file->Open()); @@ -112,7 +99,7 @@ TEST_F(HttpFileTest, BasicGet) { TEST_F(HttpFileTest, CustomHeaders) { std::vector headers{"Host: foo", "X-My-Header: Something"}; - FilePtr file(new HttpFile(HttpMethod::kGet, kTestServerReflect, + FilePtr file(new HttpFile(HttpMethod::kGet, server_.ReflectUrl(), kNoContentType, headers, kDefaultTestTimeout)); ASSERT_TRUE(file); ASSERT_TRUE(file->Open()); @@ -127,7 +114,7 @@ TEST_F(HttpFileTest, CustomHeaders) { } TEST_F(HttpFileTest, BasicPost) { - FilePtr file(new HttpFile(HttpMethod::kPost, kTestServerReflect, + FilePtr file(new HttpFile(HttpMethod::kPost, server_.ReflectUrl(), kBinaryContentType, kNoHeaders, kDefaultTestTimeout)); ASSERT_TRUE(file); @@ -161,7 +148,7 @@ TEST_F(HttpFileTest, BasicPost) { } TEST_F(HttpFileTest, BasicPut) { - FilePtr file(new HttpFile(HttpMethod::kPut, kTestServerReflect, + FilePtr file(new HttpFile(HttpMethod::kPut, server_.ReflectUrl(), kBinaryContentType, kNoHeaders, kDefaultTestTimeout)); ASSERT_TRUE(file); @@ -195,7 +182,7 @@ TEST_F(HttpFileTest, BasicPut) { } TEST_F(HttpFileTest, MultipleWrites) { - FilePtr file(new HttpFile(HttpMethod::kPut, kTestServerReflect, + FilePtr file(new HttpFile(HttpMethod::kPut, server_.ReflectUrl(), kBinaryContentType, kNoHeaders, kDefaultTestTimeout)); ASSERT_TRUE(file); @@ -239,7 +226,7 @@ TEST_F(HttpFileTest, MultipleWrites) { } TEST_F(HttpFileTest, MultipleChunks) { - FilePtr file(new HttpFile(HttpMethod::kPut, kTestServerReflect, + FilePtr file(new HttpFile(HttpMethod::kPut, server_.ReflectUrl(), kBinaryContentType, kNoHeaders, kDefaultTestTimeout)); ASSERT_TRUE(file); @@ -286,8 +273,8 @@ TEST_F(HttpFileTest, MultipleChunks) { } TEST_F(HttpFileTest, Error404) { - FilePtr file(new HttpFile(HttpMethod::kGet, kTestServer404, kNoContentType, - kNoHeaders, kDefaultTestTimeout)); + FilePtr file(new HttpFile(HttpMethod::kGet, server_.StatusCodeUrl(404), + kNoContentType, kNoHeaders, kDefaultTestTimeout)); ASSERT_TRUE(file); ASSERT_TRUE(file->Open()); @@ -301,7 +288,7 @@ TEST_F(HttpFileTest, Error404) { } TEST_F(HttpFileTest, TimeoutTriggered) { - FilePtr file(new HttpFile(HttpMethod::kGet, kTestServerLongDelay, + FilePtr file(new HttpFile(HttpMethod::kGet, server_.DelayUrl(8), kNoContentType, kNoHeaders, 1 /* timeout in seconds */)); ASSERT_TRUE(file); @@ -317,7 +304,7 @@ TEST_F(HttpFileTest, TimeoutTriggered) { } TEST_F(HttpFileTest, TimeoutNotTriggered) { - FilePtr file(new HttpFile(HttpMethod::kGet, kTestServerShortDelay, + FilePtr file(new HttpFile(HttpMethod::kGet, server_.DelayUrl(1), kNoContentType, kNoHeaders, 5 /* timeout in seconds */)); ASSERT_TRUE(file); diff --git a/packager/media/base/http_key_fetcher_unittest.cc b/packager/media/base/http_key_fetcher_unittest.cc index 179424cef8..977bd4345d 100644 --- a/packager/media/base/http_key_fetcher_unittest.cc +++ b/packager/media/base/http_key_fetcher_unittest.cc @@ -12,18 +12,6 @@ #include "packager/media/test/test_web_server.h" #include "packager/status/status_test_util.h" -namespace { -// A completely arbitrary port number, unlikely to be in use. -const int kTestServerPort = 58405; - -// Reflects back the method, body, and headers of the request as JSON. -const char kTestUrl[] = "http://localhost:58405/reflect"; -// Returns the requested HTTP status code. -const char kTestUrl404[] = "http://localhost:58405/status?code=404"; -// Returns after the requested delay. -const char kTestUrlDelayTwoSecs[] = "http://localhost:58405/delay?seconds=2"; -} // namespace - namespace shaka { namespace media { @@ -35,37 +23,37 @@ namespace media { // enough. class HttpKeyFetcherTest : public testing::Test { protected: - void SetUp() override { ASSERT_TRUE(server_.Start(kTestServerPort)); } + void SetUp() override { ASSERT_TRUE(server_.Start()); } - private: TestWebServer server_; }; TEST_F(HttpKeyFetcherTest, HttpGet) { HttpKeyFetcher fetcher; std::string response; - ASSERT_OK(fetcher.Get(kTestUrl, &response)); + ASSERT_OK(fetcher.Get(server_.ReflectUrl(), &response)); EXPECT_NE(std::string::npos, response.find("\"method\":\"GET\"")); } TEST_F(HttpKeyFetcherTest, HttpPost) { HttpKeyFetcher fetcher; std::string response; - ASSERT_OK(fetcher.Post(kTestUrl, "", &response)); + ASSERT_OK(fetcher.Post(server_.ReflectUrl(), "", &response)); EXPECT_NE(std::string::npos, response.find("\"method\":\"POST\"")); } TEST_F(HttpKeyFetcherTest, HttpFetchKeys) { HttpKeyFetcher fetcher; std::string response; - ASSERT_OK(fetcher.FetchKeys(kTestUrl, "foo=62&type=mp4", &response)); + ASSERT_OK( + fetcher.FetchKeys(server_.ReflectUrl(), "foo=62&type=mp4", &response)); EXPECT_NE(std::string::npos, response.find("\"foo=62&type=mp4\"")); } TEST_F(HttpKeyFetcherTest, InvalidUrl) { HttpKeyFetcher fetcher; std::string response; - Status status = fetcher.FetchKeys(kTestUrl404, "", &response); + Status status = fetcher.FetchKeys(server_.StatusCodeUrl(404), "", &response); EXPECT_EQ(error::HTTP_FAILURE, status.error_code()); EXPECT_NE(std::string::npos, status.error_message().find("404")); } @@ -74,7 +62,7 @@ TEST_F(HttpKeyFetcherTest, SmallTimeout) { const int32_t kTimeoutInSeconds = 1; HttpKeyFetcher fetcher(kTimeoutInSeconds); std::string response; - Status status = fetcher.FetchKeys(kTestUrlDelayTwoSecs, "", &response); + Status status = fetcher.FetchKeys(server_.DelayUrl(2), "", &response); EXPECT_EQ(error::TIME_OUT, status.error_code()); } @@ -82,7 +70,7 @@ TEST_F(HttpKeyFetcherTest, BigTimeout) { const int32_t kTimeoutInSeconds = 5; HttpKeyFetcher fetcher(kTimeoutInSeconds); std::string response; - Status status = fetcher.FetchKeys(kTestUrlDelayTwoSecs, "", &response); + Status status = fetcher.FetchKeys(server_.DelayUrl(2), "", &response); EXPECT_OK(status); } diff --git a/packager/media/test/test_web_server.cc b/packager/media/test/test_web_server.cc index 1d4fb9ac0f..ed81c55d42 100644 --- a/packager/media/test/test_web_server.cc +++ b/packager/media/test/test_web_server.cc @@ -7,6 +7,7 @@ #include "packager/media/test/test_web_server.h" #include +#include #include #include "absl/strings/numbers.h" @@ -23,6 +24,12 @@ namespace { +// A random HTTP port will be chosen, and if there is a collision, we will try +// again up to |kMaxPortTries| times. +const int kMinPortNumber = 58000; +const int kMaxPortNumber = 58999; +const int kMaxPortTries = 10; + // Get a string_view on mongoose's mg_string, which may not be nul-terminated. std::string_view MongooseStringView(const mg_str& mg_string) { return std::string_view(mg_string.ptr, mg_string.len); @@ -82,8 +89,8 @@ TestWebServer::~TestWebServer() { thread_.reset(); } -bool TestWebServer::Start(int port) { - thread_.reset(new std::thread(&TestWebServer::ThreadCallback, this, port)); +bool TestWebServer::Start() { + thread_.reset(new std::thread(&TestWebServer::ThreadCallback, this)); absl::MutexLock lock(&mutex_); while (status_ == kNew) { @@ -93,12 +100,19 @@ bool TestWebServer::Start(int port) { return status_ == kStarted; } -void TestWebServer::ThreadCallback(int port) { +bool TestWebServer::TryListenOnPort(struct mg_mgr* manager, int port) { // Mongoose needs an HTTP server address in string format. // "127.0.0.1" is "localhost", and is not visible to other machines on the // network. - std::string http_address = absl::StrFormat("http://127.0.0.1:%d", port); + base_url_ = absl::StrFormat("http://127.0.0.1:%d", port); + auto connection = + mg_http_listen(manager, base_url_.c_str(), &TestWebServer::HandleEvent, + this /* callback_data */); + return connection != NULL; +} + +void TestWebServer::ThreadCallback() { // Set up the manager structure to be automatically cleaned up when it leaves // scope. std::unique_ptr manager( @@ -106,20 +120,29 @@ void TestWebServer::ThreadCallback(int port) { // Then initialize it. mg_mgr_init(manager.get()); - auto connection = - mg_http_listen(manager.get(), http_address.c_str(), - &TestWebServer::HandleEvent, this /* callback_data */); - if (connection == NULL) { - // Failed to listen to the requested port. Mongoose has already printed an - // error message. - absl::MutexLock lock(&mutex_); - status_ = kFailed; - started_.Signal(); - return; + // Prepare to choose a random port. + std::random_device r; + std::default_random_engine e1(r()); + std::uniform_int_distribution uniform_dist(kMinPortNumber, + kMaxPortNumber); + + bool ok = false; + for (int i = 0; !ok && i < kMaxPortTries; ++i) { + // Choose a random port in the range. + int port = uniform_dist(e1); + ok = TryListenOnPort(manager.get(), port); } { absl::MutexLock lock(&mutex_); + if (!ok) { + // Failed to find a port to listen on. Mongoose has already printed an + // error message. + status_ = kFailed; + started_.Signal(); + return; + } + status_ = kStarted; started_.Signal(); } diff --git a/packager/media/test/test_web_server.h b/packager/media/test/test_web_server.h index ec5882f29f..208ab0f447 100644 --- a/packager/media/test/test_web_server.h +++ b/packager/media/test/test_web_server.h @@ -17,6 +17,7 @@ // Forward declare mongoose struct types, used as pointers below. struct mg_connection; struct mg_http_message; +struct mg_mgr; namespace shaka { namespace media { @@ -26,7 +27,20 @@ class TestWebServer { TestWebServer(); ~TestWebServer(); - bool Start(int port); + bool Start(); + + // Reflects back the request characteristics as a JSON response. + std::string ReflectUrl() { return base_url_ + "/reflect"; } + + // Responds with a specific HTTP status code. + std::string StatusCodeUrl(int code) { + return base_url_ + "/status?code=" + std::to_string(code); + } + + // Responds with HTTP 200 after a delay. + std::string DelayUrl(int seconds) { + return base_url_ + "/delay?seconds=" + std::to_string(seconds); + } private: enum TestWebServerStatus { @@ -49,7 +63,12 @@ class TestWebServer { std::unique_ptr thread_; - void ThreadCallback(int port); + std::string base_url_; + + // Sets base_url_. + bool TryListenOnPort(struct mg_mgr* manager, int port); + + void ThreadCallback(); static void HandleEvent(struct mg_connection* connection, int event,