From e3bf233c210864a52d4ad938d2dcc2c4cc6b7d1e Mon Sep 17 00:00:00 2001 From: Tor Brede Vekterli Date: Fri, 7 Jun 2019 14:06:08 +0000 Subject: Convert tests in 'common' module from CppUnit to GTest Still builds shared non-test library. --- storage/src/tests/common/CMakeLists.txt | 7 +- ...al_bucket_space_distribution_converter_test.cpp | 80 +++++++------------- storage/src/tests/common/metricstest.cpp | 88 +++++++--------------- storage/src/tests/common/storagelinktest.cpp | 44 ++++++----- storage/src/tests/common/storagelinktest.h | 40 ---------- .../filestorage/filestormanagertest.cpp | 10 ++- .../filestorage/modifiedbucketcheckertest.cpp | 2 +- .../storageserver/bucketintegritycheckertest.cpp | 16 ++-- .../src/tests/storageserver/mergethrottlertest.cpp | 6 +- 9 files changed, 105 insertions(+), 188 deletions(-) delete mode 100644 storage/src/tests/common/storagelinktest.h (limited to 'storage') 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 -#include #include #include +#include -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 #include #include #include @@ -14,15 +13,17 @@ #include #include #include +#include #include #include 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 _node; std::unique_ptr _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(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 #include +#include +#include +#include #include #include -#include using document::test::makeDocumentBucket; +using namespace ::testing; namespace storage { -CPPUNIT_TEST_SUITE_REGISTRATION(StorageLinkTest); +struct StorageLinkTest : public Test { + std::unique_ptr _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(); _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(makeDocumentBucket(document::BucketId(0)), "")); _feeder->close(); _feeder->flush(); - CPPUNIT_ASSERT_EQUAL((size_t) 1, _feeder->getNumReplies()); - CPPUNIT_ASSERT_EQUAL( - dynamic_cast( - *_feeder->getReply(0)).getResult(), + ASSERT_EQ(1, _feeder->getNumReplies()); + EXPECT_EQ( + dynamic_cast(*_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 -#include -#include - -namespace storage { - -struct StorageLinkTest : public CppUnit::TestFixture { - FastOS_ThreadPool _threadPool; - std::unique_ptr _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 -#include +#include #include #include #include @@ -2083,7 +2083,9 @@ namespace { void print(std::ostream& out, bool, const std::string&) const override { out << "MidLink"; } bool onUp(const std::shared_ptr & 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 #include -#include +#include #include #include #include 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 #include #include -#include +#include #include #include @@ -218,13 +218,13 @@ void BucketIntegrityCheckerTest::testBasicFunctionality() // Answering a message on disk with no more buckets does not trigger new std::shared_ptr 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 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 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 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 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 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( @@ -277,7 +277,7 @@ void BucketIntegrityCheckerTest::testBasicFunctionality() cmd7->getBucketId()); std::shared_ptr 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 #include #include -#include +#include #include #include #include @@ -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()); -- cgit v1.2.3