Convert tests in 'common' module from CppUnit to GTest

Still builds shared non-test library.

9 files changed, 105 insertions, 188 deletions

diff --git a/storage/src/tests/common/CMakeLists.txt b/storage/src/tests/common/CMakeLists.txt
index ce5376209dc..075dc263be9 100644
--- a/storage/src/tests/common/CMakeLists.txt
+++ b/storage/src/tests/common/CMakeLists.txt
@@ -2,9 +2,6 @@
 vespa_add_library(storage_testcommon TEST
     SOURCES
     dummystoragelink.cpp
-    global_bucket_space_distribution_converter_test.cpp
-    metricstest.cpp
-    storagelinktest.cpp
     testhelper.cpp
     testnodestateupdater.cpp
     teststorageapp.cpp
@@ -14,8 +11,12 @@ vespa_add_library(storage_testcommon TEST
 
 vespa_add_executable(storage_common_gtest_runner_app TEST
     SOURCES
+    global_bucket_space_distribution_converter_test.cpp
     gtest_runner.cpp
+    metricstest.cpp
+    storagelinktest.cpp
     DEPENDS
+    storage_testcommon
     storage
     gtest
 )
diff --git a/storage/src/tests/common/global_bucket_space_distribution_converter_test.cpp b/storage/src/tests/common/global_bucket_space_distribution_converter_test.cpp
index d75f2ac6459..2103970a8f0 100644
--- a/storage/src/tests/common/global_bucket_space_distribution_converter_test.cpp
+++ b/storage/src/tests/common/global_bucket_space_distribution_converter_test.cpp
@@ -1,37 +1,13 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include <vespa/storage/common/global_bucket_space_distribution_converter.h>
-#include <vespa/vdstestlib/cppunit/macros.h>
 #include <vespa/config/config.h>
 #include <vespa/vdslib/state/clusterstate.h>
+#include <vespa/vespalib/gtest/gtest.h>
 
-namespace storage {
-
-struct GlobalBucketSpaceDistributionConverterTest : public CppUnit::TestFixture {
-    CPPUNIT_TEST_SUITE(GlobalBucketSpaceDistributionConverterTest);
-    CPPUNIT_TEST(can_transform_flat_cluster_config);
-    CPPUNIT_TEST(can_transform_single_level_multi_group_config);
-    CPPUNIT_TEST(can_transform_multi_level_multi_group_config);
-    CPPUNIT_TEST(can_transform_heterogenous_multi_group_config);
-    CPPUNIT_TEST(can_transform_concrete_distribution_instance);
-    CPPUNIT_TEST(config_retired_state_is_propagated);
-    CPPUNIT_TEST(group_capacities_are_propagated);
-    CPPUNIT_TEST(global_distribution_has_same_owner_distributors_as_default);
-    CPPUNIT_TEST(can_generate_config_with_legacy_partition_spec);
-    CPPUNIT_TEST_SUITE_END();
+using namespace ::testing;
 
-    void can_transform_flat_cluster_config();
-    void can_transform_single_level_multi_group_config();
-    void can_transform_multi_level_multi_group_config();
-    void can_transform_heterogenous_multi_group_config();
-    void can_transform_concrete_distribution_instance();
-    void config_retired_state_is_propagated();
-    void group_capacities_are_propagated();
-    void global_distribution_has_same_owner_distributors_as_default();
-    void can_generate_config_with_legacy_partition_spec();
-};
-
-CPPUNIT_TEST_SUITE_REGISTRATION(GlobalBucketSpaceDistributionConverterTest);
+namespace storage {
 
 using DistributionConfig = vespa::config::content::StorDistributionConfig;
 
@@ -77,12 +53,12 @@ disk_distribution MODULO_BID
 
 }
 
-void GlobalBucketSpaceDistributionConverterTest::can_transform_flat_cluster_config() {
-    CPPUNIT_ASSERT_EQUAL(expected_flat_global_config, default_to_global_config(default_flat_config));
+TEST(GlobalBucketSpaceDistributionConverterTest, can_transform_flat_cluster_config) {
+    EXPECT_EQ(expected_flat_global_config, default_to_global_config(default_flat_config));
 }
 
 
-void GlobalBucketSpaceDistributionConverterTest::can_transform_single_level_multi_group_config() {
+TEST(GlobalBucketSpaceDistributionConverterTest, can_transform_single_level_multi_group_config) {
     vespalib::string default_config(
 R"(redundancy 2
 group[3]
@@ -143,10 +119,10 @@ group[2].nodes[2].index 5
 group[2].nodes[2].retired false
 disk_distribution MODULO_BID
 )");
-    CPPUNIT_ASSERT_EQUAL(expected_global_config, default_to_global_config(default_config));
+    EXPECT_EQ(expected_global_config, default_to_global_config(default_config));
 }
 
-void GlobalBucketSpaceDistributionConverterTest::can_transform_multi_level_multi_group_config() {
+TEST(GlobalBucketSpaceDistributionConverterTest, can_transform_multi_level_multi_group_config) {
     vespalib::string default_config(
 R"(redundancy 2
 group[5]
@@ -226,13 +202,13 @@ group[6].nodes[0].index 3
 group[6].nodes[0].retired false
 disk_distribution MODULO_BID
 )");
-    CPPUNIT_ASSERT_EQUAL(expected_global_config, default_to_global_config(default_config));
+    EXPECT_EQ(expected_global_config, default_to_global_config(default_config));
 }
 
 // FIXME partition specs are order-invariant with regards to groups, so heterogenous
 // setups will not produce the expected replica distribution.
 // TODO Consider disallowing entirely when using global docs.
-void GlobalBucketSpaceDistributionConverterTest::can_transform_heterogenous_multi_group_config() {
+TEST(GlobalBucketSpaceDistributionConverterTest, can_transform_heterogenous_multi_group_config) {
     vespalib::string default_config(
 R"(redundancy 2
 ready_copies 2
@@ -279,17 +255,17 @@ group[2].nodes[0].index 2
 group[2].nodes[0].retired false
 disk_distribution MODULO_BID
 )");
-    CPPUNIT_ASSERT_EQUAL(expected_global_config, default_to_global_config(default_config));
+    EXPECT_EQ(expected_global_config, default_to_global_config(default_config));
 }
 
-void GlobalBucketSpaceDistributionConverterTest::can_transform_concrete_distribution_instance() {
+TEST(GlobalBucketSpaceDistributionConverterTest, can_transform_concrete_distribution_instance) {
     auto default_cfg = GlobalBucketSpaceDistributionConverter::string_to_config(default_flat_config);
     lib::Distribution flat_distr(*default_cfg);
     auto global_distr = GlobalBucketSpaceDistributionConverter::convert_to_global(flat_distr);
-    CPPUNIT_ASSERT_EQUAL(expected_flat_global_config, global_distr->serialize());
+    EXPECT_EQ(expected_flat_global_config, global_distr->serialize());
 }
 
-void GlobalBucketSpaceDistributionConverterTest::config_retired_state_is_propagated() {
+TEST(GlobalBucketSpaceDistributionConverterTest, config_retired_state_is_propagated) {
     vespalib::string default_config(
 R"(redundancy 1
 group[1]
@@ -308,14 +284,14 @@ group[0].nodes[2].retired true
     auto default_cfg = GlobalBucketSpaceDistributionConverter::string_to_config(default_config);
     auto as_global = GlobalBucketSpaceDistributionConverter::convert_to_global(*default_cfg);
 
-    CPPUNIT_ASSERT_EQUAL(size_t(1), as_global->group.size());
-    CPPUNIT_ASSERT_EQUAL(size_t(3), as_global->group[0].nodes.size());
-    CPPUNIT_ASSERT_EQUAL(false, as_global->group[0].nodes[0].retired);
-    CPPUNIT_ASSERT_EQUAL(true, as_global->group[0].nodes[1].retired);
-    CPPUNIT_ASSERT_EQUAL(true, as_global->group[0].nodes[2].retired);
+    ASSERT_EQ(1, as_global->group.size());
+    ASSERT_EQ(3, as_global->group[0].nodes.size());
+    EXPECT_FALSE(as_global->group[0].nodes[0].retired);
+    EXPECT_TRUE(as_global->group[0].nodes[1].retired);
+    EXPECT_TRUE(as_global->group[0].nodes[2].retired);
 }
 
-void GlobalBucketSpaceDistributionConverterTest::group_capacities_are_propagated() {
+TEST(GlobalBucketSpaceDistributionConverterTest, group_capacities_are_propagated) {
     vespalib::string default_config(
 R"(redundancy 2
 group[3]
@@ -338,13 +314,13 @@ group[2].nodes[0].index 1
     auto default_cfg = GlobalBucketSpaceDistributionConverter::string_to_config(default_config);
     auto as_global = GlobalBucketSpaceDistributionConverter::convert_to_global(*default_cfg);
 
-    CPPUNIT_ASSERT_EQUAL(size_t(3), as_global->group.size());
-    CPPUNIT_ASSERT_DOUBLES_EQUAL(5.0, as_global->group[0].capacity, 0.00001);
-    CPPUNIT_ASSERT_DOUBLES_EQUAL(2.0, as_global->group[1].capacity, 0.00001);
-    CPPUNIT_ASSERT_DOUBLES_EQUAL(3.0, as_global->group[2].capacity, 0.00001);
+    ASSERT_EQ(3, as_global->group.size());
+    EXPECT_DOUBLE_EQ(5.0, as_global->group[0].capacity);
+    EXPECT_DOUBLE_EQ(2.0, as_global->group[1].capacity);
+    EXPECT_DOUBLE_EQ(3.0, as_global->group[2].capacity);
 }
 
-void GlobalBucketSpaceDistributionConverterTest::global_distribution_has_same_owner_distributors_as_default() {
+TEST(GlobalBucketSpaceDistributionConverterTest, global_distribution_has_same_owner_distributors_as_default) {
     vespalib::string default_config(
 R"(redundancy 2
 ready_copies 2
@@ -375,13 +351,13 @@ group[2].nodes[1].index 2
         document::BucketId bucket(16, i);
         const auto default_index = default_distr.getIdealDistributorNode(state, bucket, "ui");
         const auto global_index = global_distr.getIdealDistributorNode(state, bucket, "ui");
-        CPPUNIT_ASSERT_EQUAL(default_index, global_index);
+        ASSERT_EQ(default_index, global_index);
     }
 }
 
 // By "legacy" read "broken", but we need to be able to generate it to support rolling upgrades properly.
 // TODO remove on Vespa 8 - this is a workaround for https://github.com/vespa-engine/vespa/issues/8475
-void GlobalBucketSpaceDistributionConverterTest::can_generate_config_with_legacy_partition_spec() {
+TEST(GlobalBucketSpaceDistributionConverterTest, can_generate_config_with_legacy_partition_spec) {
     vespalib::string default_config(
 R"(redundancy 2
 group[3]
@@ -436,7 +412,7 @@ group[2].nodes[2].index 5
 group[2].nodes[2].retired false
 disk_distribution MODULO_BID
 )");
-    CPPUNIT_ASSERT_EQUAL(expected_global_config, default_to_global_config(default_config, true));
+    EXPECT_EQ(expected_global_config, default_to_global_config(default_config, true));
 }
 
 }
\ No newline at end of file
diff --git a/storage/src/tests/common/metricstest.cpp b/storage/src/tests/common/metricstest.cpp
index b0c82aa3166..9a9f05d500e 100644
--- a/storage/src/tests/common/metricstest.cpp
+++ b/storage/src/tests/common/metricstest.cpp
@@ -1,6 +1,5 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include <cppunit/extensions/HelperMacros.h>
 #include <vespa/document/fieldvalue/document.h>
 #include <vespa/storageapi/message/persistence.h>
 #include <vespa/storageframework/defaultimplementation/clock/fakeclock.h>
@@ -14,15 +13,17 @@
 #include <vespa/metrics/metricmanager.h>
 #include <vespa/config/common/exceptions.h>
 #include <vespa/vespalib/stllike/hash_map.hpp>
+#include <vespa/vespalib/gtest/gtest.h>
 #include <thread>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".test.metrics");
 
+using namespace ::testing;
+
 namespace storage {
 
-struct MetricsTest : public CppUnit::TestFixture {
-    FastOS_ThreadPool _threadPool;
+struct MetricsTest : public Test {
     framework::defaultimplementation::FakeClock* _clock;
     std::unique_ptr<TestServiceLayerApp> _node;
     std::unique_ptr<DummyStorageLink> _top;
@@ -40,48 +41,33 @@ struct MetricsTest : public CppUnit::TestFixture {
                                uint64_t expected);
 
     MetricsTest();
+    ~MetricsTest() override;
 
-    void setUp() override;
-    void tearDown() override;
-    void runLoad(uint32_t count = 1);
+    void SetUp() override;
+    void TearDown() override;
     void createFakeLoad();
-
-    void testFileStorMetrics();
-    void testSnapshotPresenting();
-    void testHtmlMetricsReport();
-    void testCurrentGaugeValuesOverrideSnapshotValues();
-    void testVerboseReportIncludesNonSetMetricsEvenAfterSnapshot();
-
-    CPPUNIT_TEST_SUITE(MetricsTest);
-    CPPUNIT_TEST(testFileStorMetrics);
-    CPPUNIT_TEST(testSnapshotPresenting);
-    CPPUNIT_TEST(testHtmlMetricsReport);
-    CPPUNIT_TEST(testCurrentGaugeValuesOverrideSnapshotValues);
-    CPPUNIT_TEST(testVerboseReportIncludesNonSetMetricsEvenAfterSnapshot);
-    CPPUNIT_TEST_SUITE_END();
 };
 
-CPPUNIT_TEST_SUITE_REGISTRATION(MetricsTest);
-
 namespace {
     struct MetricClock : public metrics::MetricManager::Timer
     {
         framework::Clock& _clock;
-        MetricClock(framework::Clock& c) : _clock(c) {}
+        explicit MetricClock(framework::Clock& c) : _clock(c) {}
         time_t getTime() const override { return _clock.getTimeInSeconds().getTime(); }
         time_t getTimeInMilliSecs() const override { return _clock.getTimeInMillis().getTime(); }
     };
 }
 
 MetricsTest::MetricsTest()
-    : _threadPool(256*1024),
-      _clock(0),
+    : _clock(nullptr),
       _top(),
       _metricsConsumer()
 {
 }
 
-void MetricsTest::setUp() {
+MetricsTest::~MetricsTest() = default;
+
+void MetricsTest::SetUp() {
     _config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, "metricstest"));
     assert(system(("rm -rf " + getRootFolder(*_config)).c_str()) == 0);
     try {
@@ -124,7 +110,7 @@ void MetricsTest::setUp() {
     _metricManager->init(_config->getConfigId(), _node->getThreadPool());
 }
 
-void MetricsTest::tearDown() {
+void MetricsTest::TearDown() {
     _metricManager->stop();
     _metricsConsumer.reset();
     _topSet.reset();
@@ -220,7 +206,7 @@ void MetricsTest::createFakeLoad()
     }
 }
 
-void MetricsTest::testFileStorMetrics() {
+TEST_F(MetricsTest, filestor_metrics) {
     createFakeLoad();
     std::ostringstream ost;
     framework::HttpUrlPath path("metrics?interval=-1&format=text");
@@ -244,30 +230,28 @@ void MetricsTest::testFileStorMetrics() {
     std::ostringstream ost;\
     framework::HttpUrlPath path(pathost.str()); \
     bool retVal = _metricsConsumer->reportStatus(ost, path); \
-    CPPUNIT_ASSERT_MESSAGE("_metricsConsumer->reportStatus failed", retVal); \
+    ASSERT_TRUE(retVal) << "_metricsConsumer->reportStatus failed"; \
     std::string s = ost.str(); \
     if (count == -1) { \
-        CPPUNIT_ASSERT_MESSAGE(std::string("Metric ") + metric + " was set", \
-                               s.find(metric) == std::string::npos); \
+        ASSERT_TRUE(s.find(metric) == std::string::npos) << std::string("Metric ") + metric + " was set"; \
     } else { \
         std::ostringstream valueost; \
         valueost << metric << " count=" << count; \
-        CPPUNIT_ASSERT_MESSAGE("Did not find value " + valueost.str() \
-                                    + " in metric dump " + s, \
-                               s.find(valueost.str()) != std::string::npos); \
+        ASSERT_TRUE(s.find(valueost.str()) != std::string::npos) \
+                << "Did not find value " + valueost.str() + " in metric dump " + s; \
     } \
 }
 
-void MetricsTest::testSnapshotPresenting() {
+TEST_F(MetricsTest, snapshot_presenting) {
     FileStorDiskMetrics& disk0(*_filestorMetrics->disks[0]);
     FileStorThreadMetrics& thread0(*disk0.threads[0]);
 
-    LOG(info, "Adding to get metric");
+    LOG(debug, "Adding to get metric");
 
     using documentapi::LoadType;
     thread0.get[LoadType::DEFAULT].count.inc(1);
 
-    LOG(info, "Waiting for 5 minute snapshot to be taken");
+    LOG(debug, "Waiting for 5 minute snapshot to be taken");
     // Wait until active metrics have been added to 5 min snapshot and reset
     for (uint32_t i=0; i<6; ++i) {
         _clock->addSecondsToTime(60);
@@ -279,7 +263,7 @@ void MetricsTest::testSnapshotPresenting() {
             FastOS_Thread::Sleep(1);
         }
     }
-    LOG(info, "5 minute snapshot should have been taken. Adding put count");
+    LOG(debug, "5 minute snapshot should have been taken. Adding put count");
 
     thread0.put[LoadType::DEFAULT].count.inc(1);
 
@@ -300,7 +284,7 @@ void MetricsTest::testSnapshotPresenting() {
     ASSERT_METRIC(-1, "vds.filestor.alldisks.allthreads.get.sum.count", 1);
 }
 
-void MetricsTest::testHtmlMetricsReport() {
+TEST_F(MetricsTest, html_metrics_report) {
     createFakeLoad();
     _clock->addSecondsToTime(6 * 60);
     _metricManager->timeChangedNotification();
@@ -309,16 +293,7 @@ void MetricsTest::testHtmlMetricsReport() {
     std::ostringstream ost;
     framework::HttpUrlPath path("metrics?interval=300&format=html");
     bool retVal = _metricsConsumer->reportStatus(ost, path);
-    CPPUNIT_ASSERT_MESSAGE("_metricsConsumer->reportStatus failed", retVal);
-    std::string s = ost.str();
-    // Not actually testing against content. Better to manually verify that
-    // HTML look sane after changes.
-    //std::cerr << s << "\n";
-    {
-        std::ofstream out("metricsreport.html");
-        out << s;
-        out.close();
-    }
+    ASSERT_TRUE(retVal) << "_metricsConsumer->reportStatus failed";
 }
 
 void
@@ -332,13 +307,12 @@ MetricsTest::assertMetricLastValue(const std::string& name,
          << "&verbosity=2";
     std::ostringstream report;
     framework::HttpUrlPath uri(path.str());
-    CPPUNIT_ASSERT(_metricsConsumer->reportStatus(report, uri));
+    ASSERT_TRUE(_metricsConsumer->reportStatus(report, uri));
     std::ostringstream expectedSubstr;
     expectedSubstr << " last=" << expected;
     auto str = report.str();
-    CPPUNIT_ASSERT_MESSAGE("Did not find value " + expectedSubstr.str()
-                           + " in metric dump " + str,
-                           str.find(expectedSubstr.str()) != std::string::npos);
+    ASSERT_TRUE(str.find(expectedSubstr.str()) != std::string::npos)
+            << "Did not find value " + expectedSubstr.str() + " in metric dump " + str;
 }
 
 using namespace std::chrono_literals;
@@ -355,9 +329,7 @@ MetricsTest::createSnapshotForPeriod(std::chrono::seconds secs)
     }
 }
 
-void
-MetricsTest::testCurrentGaugeValuesOverrideSnapshotValues()
-{
+TEST_F(MetricsTest, current_gauge_values_override_snapshot_values) {
     auto& metrics(*_bucketManagerMetrics->disks[0]);
     metrics.docs.set(1000);
     // Take a 5 minute snapshot of active metrics (1000 docs).
@@ -371,9 +343,7 @@ MetricsTest::testCurrentGaugeValuesOverrideSnapshotValues()
     assertMetricLastValue("vds.datastored.alldisks.docs", -1, 2000);
 }
 
-void
-MetricsTest::testVerboseReportIncludesNonSetMetricsEvenAfterSnapshot()
-{
+TEST_F(MetricsTest, verbose_report_includes_non_set_metrics_even_after_snapshot) {
     createSnapshotForPeriod(5min);
     // When using verbosity=2 (which is what the system test framework invokes),
     // all metrics should be included regardless of whether they've been set or
diff --git a/storage/src/tests/common/storagelinktest.cpp b/storage/src/tests/common/storagelinktest.cpp
index 95fe8ad23da..890376debbc 100644
--- a/storage/src/tests/common/storagelinktest.cpp
+++ b/storage/src/tests/common/storagelinktest.cpp
@@ -1,25 +1,35 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include <tests/common/storagelinktest.h>
 #include <vespa/document/test/make_document_bucket.h>
+#include <tests/common/dummystoragelink.h>
+#include <vespa/storageapi/message/stat.h>
+#include <vespa/vespalib/gtest/gtest.h>
 #include <iostream>
 #include <string>
-#include <vespa/storageapi/message/stat.h>
 
 using document::test::makeDocumentBucket;
+using namespace ::testing;
 
 namespace storage {
 
-CPPUNIT_TEST_SUITE_REGISTRATION(StorageLinkTest);
+struct StorageLinkTest : public Test {
+    std::unique_ptr<DummyStorageLink> _feeder;
+    DummyStorageLink* _middle;
+    DummyStorageLink* _replier;
+
+    StorageLinkTest();
+
+    void SetUp() override;
+};
 
 StorageLinkTest::StorageLinkTest()
-    : _threadPool(1024),
-      _feeder(),
-      _middle(0),
-      _replier(0) {}
+    : _feeder(),
+      _middle(nullptr),
+      _replier(nullptr)
+{}
 
-void StorageLinkTest::setUp() {
-    _feeder.reset(new DummyStorageLink());
+void StorageLinkTest::SetUp() {
+    _feeder = std::make_unique<DummyStorageLink>();
     _middle = new DummyStorageLink();
     _replier = new DummyStorageLink();
     _feeder->push_back(StorageLink::UP(_middle));
@@ -27,7 +37,7 @@ void StorageLinkTest::setUp() {
     _replier->setAutoreply(true);
 }
 
-void StorageLinkTest::testPrinting() {
+TEST_F(StorageLinkTest, printing) {
     std::ostringstream actual;
     actual << *_feeder;
     std::string expected =
@@ -36,21 +46,19 @@ void StorageLinkTest::testPrinting() {
 "  DummyStorageLink(autoreply = off, dispatch = off, 0 commands, 0 replies)\n"
 "  DummyStorageLink(autoreply = on, dispatch = off, 0 commands, 0 replies)";
 
-    CPPUNIT_ASSERT_EQUAL(expected, actual.str());
+    EXPECT_EQ(expected, actual.str());
 }
 
-void StorageLinkTest::testNotImplemented() {
+TEST_F(StorageLinkTest, not_implemented) {
     _feeder->open();
       // Test that a message that nobody handles fails with NOT_IMPLEMENTED
     _replier->setIgnore(true);
-    _feeder->sendDown(api::StorageCommand::SP(
-                new api::StatBucketCommand(makeDocumentBucket(document::BucketId(0)), "")));
+    _feeder->sendDown(std::make_shared<api::StatBucketCommand>(makeDocumentBucket(document::BucketId(0)), ""));
     _feeder->close();
     _feeder->flush();
-    CPPUNIT_ASSERT_EQUAL((size_t) 1, _feeder->getNumReplies());
-    CPPUNIT_ASSERT_EQUAL(
-            dynamic_cast<api::StatBucketReply&>(
-                *_feeder->getReply(0)).getResult(),
+    ASSERT_EQ(1, _feeder->getNumReplies());
+    EXPECT_EQ(
+            dynamic_cast<api::StatBucketReply&>(*_feeder->getReply(0)).getResult(),
             api::ReturnCode(api::ReturnCode::NOT_IMPLEMENTED, "Statbucket"));
     _feeder->reset();
     _replier->setIgnore(false);
diff --git a/storage/src/tests/common/storagelinktest.h b/storage/src/tests/common/storagelinktest.h
deleted file mode 100644
index 6f62edefe33..00000000000
--- a/storage/src/tests/common/storagelinktest.h
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-
-#pragma once
-
-#include <cppunit/extensions/HelperMacros.h>
-#include <tests/common/dummystoragelink.h>
-#include <vespa/fastos/thread.h>
-
-namespace storage {
-
-struct StorageLinkTest : public CppUnit::TestFixture {
-    FastOS_ThreadPool _threadPool;
-    std::unique_ptr<DummyStorageLink> _feeder;
-    DummyStorageLink* _middle;
-    DummyStorageLink* _replier;
-
-    StorageLinkTest();
-
-    void setUp() override;
-
-    void testPrinting();
-    void testNotImplemented();
-
-    static bool callOnUp(StorageLink& link, const api::StorageMessage::SP& msg) {
-        return link.onUp(msg);
-    }
-    static bool callOnDown(StorageLink& link, const api::StorageMessage::SP& msg) {
-        return link.onDown(msg);
-    }
-    static void callOnFlush(StorageLink& link, bool downwards) {
-        link.onFlush(downwards);
-    }
-
-    CPPUNIT_TEST_SUITE(StorageLinkTest);
-    CPPUNIT_TEST(testPrinting);
-    CPPUNIT_TEST(testNotImplemented);
-    CPPUNIT_TEST_SUITE_END();
-};
-
-}
diff --git a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp
index c9285442fbc..3888b7503e2 100644
--- a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp
+++ b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp
@@ -1,7 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include <tests/common/testhelper.h>
-#include <tests/common/storagelinktest.h>
+#include <tests/common/dummystoragelink.h>
 #include <tests/common/teststorageapp.h>
 #include <tests/persistence/filestorage/forwardingmessagesender.h>
 #include <vespa/document/repo/documenttyperepo.h>
@@ -2083,7 +2083,9 @@ namespace {
 
         void print(std::ostream& out, bool, const std::string&) const override { out << "MidLink"; }
         bool onUp(const std::shared_ptr<api::StorageMessage> & msg) override {
-            if (!StorageLinkTest::callOnUp(_up, msg)) _up.sendUp(msg);
+            if (!_up.onUp(msg)) {
+                _up.sendUp(msg);
+            }
             return true;
         }
 
@@ -2126,13 +2128,13 @@ namespace {
                 if ((address == _leftAddr && !msg->getType().isReply()) ||
                     (address == _rightAddr && msg->getType().isReply()))
                 {
-                    if (!StorageLinkTest::callOnDown(_left, msg)) {
+                    if (!_left.onDown(msg)) {
                         _left.sendDown(msg);
                     }
                 } else if ((address == _rightAddr && !msg->getType().isReply()) ||
                            (address == _leftAddr && msg->getType().isReply()))
                 {
-                    if (!StorageLinkTest::callOnDown(_right, msg)) {
+                    if (!_right.onDown(msg)) {
                         _right.sendDown(msg);
                     }
                 } else {
diff --git a/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp b/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp
index 7895d9e4cd0..e6412ba9fd1 100644
--- a/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp
+++ b/storage/src/tests/persistence/filestorage/modifiedbucketcheckertest.cpp
@@ -2,7 +2,7 @@
 
 #include <vespa/vdstestlib/cppunit/macros.h>
 #include <tests/common/testhelper.h>
-#include <tests/common/storagelinktest.h>
+#include <tests/common/dummystoragelink.h>
 #include <tests/common/teststorageapp.h>
 #include <vespa/persistence/dummyimpl/dummypersistence.h>
 #include <vespa/storage/persistence/filestorage/modifiedbucketchecker.h>
diff --git a/storage/src/tests/storageserver/bucketintegritycheckertest.cpp b/storage/src/tests/storageserver/bucketintegritycheckertest.cpp
index 4ff980b7950..a4a6cbae9cf 100644
--- a/storage/src/tests/storageserver/bucketintegritycheckertest.cpp
+++ b/storage/src/tests/storageserver/bucketintegritycheckertest.cpp
@@ -8,7 +8,7 @@
 #include <vespa/storage/storageserver/bucketintegritychecker.h>
 #include <vespa/storageapi/message/persistence.h>
 #include <tests/common/testhelper.h>
-#include <tests/common/storagelinktest.h>
+#include <tests/common/dummystoragelink.h>
 #include <vespa/vespalib/io/fileutil.h>
 #include <tests/common/teststorageapp.h>
 
@@ -218,13 +218,13 @@ void BucketIntegrityCheckerTest::testBasicFunctionality()
         // Answering a message on disk with no more buckets does not trigger new
         std::shared_ptr<RepairBucketReply> reply1(
                 new RepairBucketReply(*cmd3));
-        CPPUNIT_ASSERT(StorageLinkTest::callOnUp(checker, reply1));
+        CPPUNIT_ASSERT(checker.onUp(reply1));
         FastOS_Thread::Sleep(10); // Give next message chance to come
         ASSERT_COMMAND_COUNT(4, *dummyLink);
         // Answering a message on disk with more buckets trigger new repair
         std::shared_ptr<RepairBucketReply> reply2(
                 new RepairBucketReply(*cmd2));
-        CPPUNIT_ASSERT(StorageLinkTest::callOnUp(checker, reply2));
+        CPPUNIT_ASSERT(checker.onUp(reply2));
         dummyLink->waitForMessages(5, _timeout);
         FastOS_Thread::Sleep(10); // Give 6th message chance to come
         ASSERT_COMMAND_COUNT(5, *dummyLink);
@@ -238,7 +238,7 @@ void BucketIntegrityCheckerTest::testBasicFunctionality()
         std::shared_ptr<RepairBucketReply> reply3(
                 new RepairBucketReply(*cmd1));
         reply3->setResult(api::ReturnCode(api::ReturnCode::IGNORED));
-        CPPUNIT_ASSERT(StorageLinkTest::callOnUp(checker, reply3));
+        CPPUNIT_ASSERT(checker.onUp(reply3));
         dummyLink->waitForMessages(6, _timeout);
         FastOS_Thread::Sleep(10); // Give 7th message chance to come
         ASSERT_COMMAND_COUNT(6, *dummyLink);
@@ -252,7 +252,7 @@ void BucketIntegrityCheckerTest::testBasicFunctionality()
         std::shared_ptr<RepairBucketReply> reply4(
                 new RepairBucketReply(*cmd4));
         reply3->setResult(api::ReturnCode(api::ReturnCode::BUCKET_NOT_FOUND));
-        CPPUNIT_ASSERT(StorageLinkTest::callOnUp(checker, reply4));
+        CPPUNIT_ASSERT(checker.onUp(reply4));
         FastOS_Thread::Sleep(10); // Give 7th message chance to come
         ASSERT_COMMAND_COUNT(6, *dummyLink);
 
@@ -261,13 +261,13 @@ void BucketIntegrityCheckerTest::testBasicFunctionality()
         std::shared_ptr<RepairBucketReply> reply5(
                 new RepairBucketReply(*cmd5, newInfo));
         reply5->setAltered(true);
-        CPPUNIT_ASSERT(StorageLinkTest::callOnUp(checker, reply5));
+        CPPUNIT_ASSERT(checker.onUp(reply5));
 
         // Finish run. New iteration should not start yet as min
         // cycle time has not passed
         std::shared_ptr<RepairBucketReply> reply6(
                 new RepairBucketReply(*cmd6));
-        CPPUNIT_ASSERT(StorageLinkTest::callOnUp(checker, reply6));
+        CPPUNIT_ASSERT(checker.onUp(reply6));
         dummyLink->waitForMessages(7, _timeout);
         ASSERT_COMMAND_COUNT(7, *dummyLink);
         RepairBucketCommand *cmd7 = dynamic_cast<RepairBucketCommand*>(
@@ -277,7 +277,7 @@ void BucketIntegrityCheckerTest::testBasicFunctionality()
                              cmd7->getBucketId());
         std::shared_ptr<RepairBucketReply> reply7(
                 new RepairBucketReply(*cmd7));
-        CPPUNIT_ASSERT(StorageLinkTest::callOnUp(checker, reply7));
+        CPPUNIT_ASSERT(checker.onUp(reply7));
         FastOS_Thread::Sleep(10); // Give 8th message chance to come
         ASSERT_COMMAND_COUNT(7, *dummyLink);
 
diff --git a/storage/src/tests/storageserver/mergethrottlertest.cpp b/storage/src/tests/storageserver/mergethrottlertest.cpp
index 3d469fc4252..5815bbd67c8 100644
--- a/storage/src/tests/storageserver/mergethrottlertest.cpp
+++ b/storage/src/tests/storageserver/mergethrottlertest.cpp
@@ -3,7 +3,7 @@
 #include <vespa/vespalib/util/document_runnable.h>
 #include <vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h>
 #include <tests/common/testhelper.h>
-#include <tests/common/storagelinktest.h>
+#include <tests/common/dummystoragelink.h>
 #include <tests/common/teststorageapp.h>
 #include <tests/common/dummystoragelink.h>
 #include <vespa/document/test/make_document_bucket.h>
@@ -595,7 +595,7 @@ MergeThrottlerTest::test42DistributorBehaviorDoesNotTakeOwnership()
 
     // Flush throttler (synchronously). Should NOT generate a reply
     // for the merge command, as it is not owned by the throttler
-    StorageLinkTest::callOnFlush(*_throttlers[1], true);
+    _throttlers[1]->onFlush(true);
 
     CPPUNIT_ASSERT_EQUAL(std::size_t(0), _bottomLinks[1]->getNumCommands());
     CPPUNIT_ASSERT_EQUAL(std::size_t(0), _topLinks[1]->getNumReplies());
@@ -653,7 +653,7 @@ MergeThrottlerTest::testEndOfChainExecutionDoesNotTakeOwnership()
 
     // Flush throttler (synchronously). Should NOT generate a reply
     // for the merge command, as it is not owned by the throttler
-    StorageLinkTest::callOnFlush(*_throttlers[2], true);
+    _throttlers[2]->onFlush(true);
 
     CPPUNIT_ASSERT_EQUAL(std::size_t(0), _bottomLinks[2]->getNumCommands());
     CPPUNIT_ASSERT_EQUAL(std::size_t(0), _topLinks[2]->getNumReplies());