Rename LibcurlHttpFetcherTest Both libcurl_http_fetcher_unittest.cc and http_fetcher_unittest.cc have a class named "LibcurlHttpFetcherTest", and they do different things. This is an ODR violation that took ~2 days to investigate. Fix by renaming + moving classes to anon namespace. Test: th Change-Id: I2271b3563a532f225b1efa4070e098246075e8da
diff --git a/common/http_fetcher_unittest.cc b/common/http_fetcher_unittest.cc index 99ea99b..bc8a325 100644 --- a/common/http_fetcher_unittest.cc +++ b/common/http_fetcher_unittest.cc
@@ -97,6 +97,8 @@ // Class hierarchy for HTTP server implementations. // +namespace { + class HttpServer { public: // This makes it an abstract class (dirty but works). @@ -193,10 +195,10 @@ // Class hierarchy for HTTP fetcher test wrappers. // -class AnyHttpFetcherTest { +class AnyHttpFetcherFactory { public: - AnyHttpFetcherTest() {} - virtual ~AnyHttpFetcherTest() {} + AnyHttpFetcherFactory() {} + virtual ~AnyHttpFetcherFactory() {} virtual HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) = 0; HttpFetcher* NewLargeFetcher(size_t num_proxies) { @@ -231,10 +233,10 @@ FakeHardware fake_hardware_; }; -class MockHttpFetcherTest : public AnyHttpFetcherTest { +class MockHttpFetcherFactory : public AnyHttpFetcherFactory { public: // Necessary to unhide the definition in the base class. - using AnyHttpFetcherTest::NewLargeFetcher; + using AnyHttpFetcherFactory::NewLargeFetcher; HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) override { brillo::Blob big_data(1000000); return new MockHttpFetcher( @@ -242,7 +244,7 @@ } // Necessary to unhide the definition in the base class. - using AnyHttpFetcherTest::NewSmallFetcher; + using AnyHttpFetcherFactory::NewSmallFetcher; HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override { return new MockHttpFetcher("x", 1, proxy_resolver); } @@ -255,10 +257,10 @@ HttpServer* CreateServer() override { return new NullHttpServer; } }; -class LibcurlHttpFetcherTest : public AnyHttpFetcherTest { +class LibcurlHttpFetcherFactory : public AnyHttpFetcherFactory { public: // Necessary to unhide the definition in the base class. - using AnyHttpFetcherTest::NewLargeFetcher; + using AnyHttpFetcherFactory::NewLargeFetcher; HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) override { LibcurlHttpFetcher* ret = new LibcurlHttpFetcher(proxy_resolver, &fake_hardware_); @@ -270,7 +272,7 @@ } // Necessary to unhide the definition in the base class. - using AnyHttpFetcherTest::NewSmallFetcher; + using AnyHttpFetcherFactory::NewSmallFetcher; HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override { return NewLargeFetcher(proxy_resolver); } @@ -298,10 +300,10 @@ HttpServer* CreateServer() override { return new PythonHttpServer; } }; -class MultiRangeHttpFetcherTest : public LibcurlHttpFetcherTest { +class MultiRangeHttpFetcherFactory : public LibcurlHttpFetcherFactory { public: // Necessary to unhide the definition in the base class. - using AnyHttpFetcherTest::NewLargeFetcher; + using AnyHttpFetcherFactory::NewLargeFetcher; HttpFetcher* NewLargeFetcher(ProxyResolver* proxy_resolver) override { MultiRangeHttpFetcher* ret = new MultiRangeHttpFetcher( new LibcurlHttpFetcher(proxy_resolver, &fake_hardware_)); @@ -315,7 +317,7 @@ } // Necessary to unhide the definition in the base class. - using AnyHttpFetcherTest::NewSmallFetcher; + using AnyHttpFetcherFactory::NewSmallFetcher; HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override { return NewLargeFetcher(proxy_resolver); } @@ -323,16 +325,16 @@ bool IsMulti() const override { return true; } }; -class FileFetcherTest : public AnyHttpFetcherTest { +class FileFetcherFactory : public AnyHttpFetcherFactory { public: // Necessary to unhide the definition in the base class. - using AnyHttpFetcherTest::NewLargeFetcher; + using AnyHttpFetcherFactory::NewLargeFetcher; HttpFetcher* NewLargeFetcher(ProxyResolver* /* proxy_resolver */) override { return new FileFetcher(); } // Necessary to unhide the definition in the base class. - using AnyHttpFetcherTest::NewSmallFetcher; + using AnyHttpFetcherFactory::NewSmallFetcher; HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override { return NewLargeFetcher(proxy_resolver); } @@ -372,10 +374,10 @@ ScopedTempFile temp_file_{"ue_file_fetcher.XXXXXX"}; }; -class MultiRangeHttpFetcherOverFileFetcherTest : public FileFetcherTest { +class MultiRangeHttpFetcherOverFileFetcherFactory : public FileFetcherFactory { public: // Necessary to unhide the definition in the base class. - using AnyHttpFetcherTest::NewLargeFetcher; + using AnyHttpFetcherFactory::NewLargeFetcher; HttpFetcher* NewLargeFetcher(ProxyResolver* /* proxy_resolver */) override { MultiRangeHttpFetcher* ret = new MultiRangeHttpFetcher(new FileFetcher()); ret->ClearRanges(); @@ -389,7 +391,7 @@ } // Necessary to unhide the definition in the base class. - using AnyHttpFetcherTest::NewSmallFetcher; + using AnyHttpFetcherFactory::NewSmallFetcher; HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override { return NewLargeFetcher(proxy_resolver); } @@ -427,22 +429,21 @@ private: static void TypeConstraint(T* a) { - AnyHttpFetcherTest* b = a; + AnyHttpFetcherFactory* b = a; if (b == 0) // Silence compiler warning of unused variable. *b = a; } }; // Test case types list. -typedef ::testing::Types<LibcurlHttpFetcherTest, - MockHttpFetcherTest, - MultiRangeHttpFetcherTest, - FileFetcherTest, - MultiRangeHttpFetcherOverFileFetcherTest> +typedef ::testing::Types<LibcurlHttpFetcherFactory, + MockHttpFetcherFactory, + MultiRangeHttpFetcherFactory, + FileFetcherFactory, + MultiRangeHttpFetcherOverFileFetcherFactory> HttpFetcherTestTypes; TYPED_TEST_CASE(HttpFetcherTest, HttpFetcherTestTypes); -namespace { class HttpFetcherTestDelegate : public HttpFetcherDelegate { public: HttpFetcherTestDelegate() = default; @@ -487,7 +488,6 @@ void StartTransfer(HttpFetcher* http_fetcher, const string& url) { http_fetcher->BeginTransfer(url); } -} // namespace TYPED_TEST(HttpFetcherTest, SimpleTest) { HttpFetcherTestDelegate delegate; @@ -592,7 +592,6 @@ EXPECT_EQ(string::npos, delegate.data.find("X-Bar: I do not")); } -namespace { class PausingHttpFetcherTestDelegate : public HttpFetcherDelegate { public: bool ReceivedBytes(HttpFetcher* fetcher, @@ -626,7 +625,6 @@ base::Bind(&UnpausingTimeoutCallback, delegate, my_id), base::TimeDelta::FromMilliseconds(200)); } -} // namespace TYPED_TEST(HttpFetcherTest, PauseTest) { PausingHttpFetcherTestDelegate delegate; @@ -673,7 +671,6 @@ proxy_callback.Run({1, kNoProxy}); } -namespace { class AbortingHttpFetcherTestDelegate : public HttpFetcherDelegate { public: bool ReceivedBytes(HttpFetcher* fetcher, @@ -716,7 +713,6 @@ *my_id = MessageLoop::kTaskIdNull; } } -} // namespace TYPED_TEST(HttpFetcherTest, AbortTest) { AbortingHttpFetcherTestDelegate delegate; @@ -767,7 +763,6 @@ EXPECT_EQ(1, delegate.times_transfer_terminated_called_); } -namespace { class FlakyHttpFetcherTestDelegate : public HttpFetcherDelegate { public: bool ReceivedBytes(HttpFetcher* fetcher, @@ -784,7 +779,6 @@ void TransferTerminated(HttpFetcher* fetcher) override { ADD_FAILURE(); } string data; }; -} // namespace TYPED_TEST(HttpFetcherTest, FlakyTest) { if (this->test_.IsMock() || !this->test_.IsHttpSupported()) @@ -818,7 +812,6 @@ } } -namespace { // This delegate kills the server attached to it after receiving any bytes. // This can be used for testing what happens when you try to fetch data and // the server dies. @@ -859,7 +852,6 @@ int times_transfer_terminated_called_{0}; int times_transfer_complete_called_{0}; }; -} // namespace TYPED_TEST(HttpFetcherTest, FailureTest) { // This test ensures that a fetcher responds correctly when a server isn't @@ -1000,7 +992,6 @@ EXPECT_TRUE(timeout); } -namespace { const HttpResponseCode kRedirectCodes[] = {kHttpResponseMovedPermanently, kHttpResponseFound, kHttpResponseSeeOther, @@ -1055,7 +1046,6 @@ } } } -} // namespace TYPED_TEST(HttpFetcherTest, SimpleRedirectTest) { if (this->test_.IsMock() || !this->test_.IsHttpSupported()) @@ -1103,7 +1093,6 @@ RedirectTest(server.get(), false, url, this->test_.NewLargeFetcher()); } -namespace { class MultiHttpFetcherTestDelegate : public HttpFetcherDelegate { public: explicit MultiHttpFetcherTestDelegate(int expected_response_code) @@ -1173,7 +1162,6 @@ EXPECT_EQ(expected_prefix, string(delegate.data.data(), expected_prefix.size())); } -} // namespace TYPED_TEST(HttpFetcherTest, MultiHttpFetcherSimpleTest) { if (!this->test_.IsMulti()) @@ -1323,7 +1311,6 @@ kHttpResponseUndefined); } -namespace { // This HttpFetcherDelegate calls TerminateTransfer at a configurable point. class MultiHttpFetcherTerminateTestDelegate : public HttpFetcherDelegate { public: @@ -1365,7 +1352,6 @@ size_t bytes_downloaded_{0}; size_t terminate_trigger_bytes_; }; -} // namespace TYPED_TEST(HttpFetcherTest, MultiHttpFetcherTerminateBetweenRangesTest) { if (!this->test_.IsMulti()) @@ -1396,7 +1382,6 @@ EXPECT_EQ(kRangeTrigger, delegate.bytes_downloaded_); } -namespace { class BlockedTransferTestDelegate : public HttpFetcherDelegate { public: bool ReceivedBytes(HttpFetcher* fetcher, @@ -1412,7 +1397,7 @@ void TransferTerminated(HttpFetcher* fetcher) override { ADD_FAILURE(); } }; -void BlockedTransferTestHelper(AnyHttpFetcherTest* fetcher_test, +void BlockedTransferTestHelper(AnyHttpFetcherFactory* fetcher_test, bool is_official_build) { if (fetcher_test->IsMock() || fetcher_test->IsMulti()) return; @@ -1436,7 +1421,6 @@ fetcher_test->SmallUrl(server->GetPort())))); MessageLoop::current()->Run(); } -} // namespace TYPED_TEST(HttpFetcherTest, BlockedTransferTest) { BlockedTransferTestHelper(&this->test_, false); @@ -1446,4 +1430,6 @@ BlockedTransferTestHelper(&this->test_, true); } +} // namespace + } // namespace chromeos_update_engine
diff --git a/libcurl_http_fetcher_unittest.cc b/libcurl_http_fetcher_unittest.cc index 5d67570..3543870 100644 --- a/libcurl_http_fetcher_unittest.cc +++ b/libcurl_http_fetcher_unittest.cc
@@ -31,8 +31,8 @@ namespace chromeos_update_engine { namespace { + constexpr char kHeaderName[] = "X-Goog-Test-Header"; -} class LibcurlHttpFetcherTest : public ::testing::Test { protected: @@ -48,6 +48,8 @@ UnresolvedHostStateMachine state_machine_; }; +} // namespace + TEST_F(LibcurlHttpFetcherTest, GetEmptyHeaderValueTest) { const string header_value = ""; string actual_header_value;