1 files changed, 115 insertions, 254 deletions

diff --git a/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp b/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp
index 98ad8761736..2f091572ed2 100644
--- a/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp
+++ b/storage/src/tests/storageserver/changedbucketownershiphandlertest.cpp
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include <vespa/document/base/testdocman.h>
-#include <vespa/vdstestlib/cppunit/macros.h>
 #include <vespa/storage/bucketdb/storbucketdb.h>
 #include <vespa/storage/persistence/messages.h>
 #include <vespa/storageapi/message/state.h>
@@ -16,46 +15,20 @@
 #include <vespa/document/update/documentupdate.h>
 #include <vespa/document/test/make_document_bucket.h>
 #include <vespa/storage/storageserver/changedbucketownershiphandler.h>
+#include <vespa/vespalib/gtest/gtest.h>
 
 using document::test::makeDocumentBucket;
+using namespace ::testing;
 
 namespace storage {
 
-class ChangedBucketOwnershipHandlerTest : public CppUnit::TestFixture
-{
+struct ChangedBucketOwnershipHandlerTest : Test {
     std::unique_ptr<TestServiceLayerApp> _app;
     std::unique_ptr<DummyStorageLink> _top;
     ChangedBucketOwnershipHandler* _handler;
     DummyStorageLink* _bottom;
     document::TestDocMan _testDocRepo;
 
-    CPPUNIT_TEST_SUITE(ChangedBucketOwnershipHandlerTest);
-    CPPUNIT_TEST(testEnumerateBucketsBelongingOnChangedNodes);
-    CPPUNIT_TEST(testNoPreExistingClusterState);
-    CPPUNIT_TEST(testNoAvailableDistributorsInCurrentState);
-    CPPUNIT_TEST(testNoAvailableDistributorsInCurrentAndNewState);
-    CPPUNIT_TEST(testDownEdgeToNoAvailableDistributors);
-    CPPUNIT_TEST(testOwnershipChangedOnDistributorUpEdge);
-    CPPUNIT_TEST(testDistributionConfigChangeUpdatesOwnership);
-    CPPUNIT_TEST(testAbortOpsWhenNoClusterStateSet);
-    CPPUNIT_TEST(testAbortOutdatedSplit);
-    CPPUNIT_TEST(testAbortOutdatedJoin);
-    CPPUNIT_TEST(testAbortOutdatedSetBucketState);
-    CPPUNIT_TEST(testAbortOutdatedCreateBucket);
-    CPPUNIT_TEST(testAbortOutdatedDeleteBucket);
-    CPPUNIT_TEST(testAbortOutdatedMergeBucket);
-    CPPUNIT_TEST(testAbortOutdatedRemoveLocation);
-    CPPUNIT_TEST(testIdealStateAbortsAreConfigurable);
-    CPPUNIT_TEST(testAbortOutdatedPutOperation);
-    CPPUNIT_TEST(testAbortOutdatedUpdateCommand);
-    CPPUNIT_TEST(testAbortOutdatedRemoveCommand);
-    CPPUNIT_TEST(testAbortOutdatedRevertCommand);
-    CPPUNIT_TEST(testIdealStateAbortUpdatesMetric);
-    CPPUNIT_TEST(testExternalLoadOpAbortUpdatesMetric);
-    CPPUNIT_TEST(testExternalLoadOpAbortsAreConfigurable);
-    CPPUNIT_TEST(testAbortCommandsWhenStorageNodeIsDown);
-    CPPUNIT_TEST_SUITE_END();
-
     // TODO test: down edge triggered on cluster state with cluster down?
 
     std::vector<document::BucketId> insertBuckets(
@@ -89,10 +62,10 @@ class ChangedBucketOwnershipHandlerTest : public CppUnit::TestFixture
     void sendAndExpectAbortedCreateBucket(uint16_t fromDistributorIndex);
 
     template <typename MsgType, typename... MsgParams>
-    bool changeAbortsMessage(MsgParams&&... params);
+    void expectChangeAbortsMessage(bool expected, MsgParams&& ... params);
 
     template <typename MsgType, typename... MsgParams>
-    bool downAbortsMessage(MsgParams&&... params);
+    void expectDownAbortsMessage(bool expected, MsgParams&& ... params);
 
     lib::ClusterState getDefaultTestClusterState() const {
         return lib::ClusterState("distributor:4 storage:1");
@@ -101,36 +74,9 @@ class ChangedBucketOwnershipHandlerTest : public CppUnit::TestFixture
     lib::ClusterState getStorageDownTestClusterState() const {
         return lib::ClusterState("distributor:4 storage:1 .0.s:d");
     }
-public:
-    void testEnumerateBucketsBelongingOnChangedNodes();
-    void testNoPreExistingClusterState();
-    void testNoAvailableDistributorsInCurrentState();
-    void testNoAvailableDistributorsInCurrentAndNewState();
-    void testDownEdgeToNoAvailableDistributors();
-    void testOwnershipChangedOnDistributorUpEdge();
-    void testDistributionConfigChangeUpdatesOwnership();
-    void testAbortOpsWhenNoClusterStateSet();
-    void testAbortOutdatedSplit();
-    void testAbortOutdatedJoin();
-    void testAbortOutdatedSetBucketState();
-    void testAbortOutdatedCreateBucket();
-    void testAbortOutdatedDeleteBucket();
-    void testAbortOutdatedMergeBucket();
-    void testAbortOutdatedRemoveLocation();
-    void testIdealStateAbortsAreConfigurable();
-    void testAbortOutdatedPutOperation();
-    void testAbortOutdatedUpdateCommand();
-    void testAbortOutdatedRemoveCommand();
-    void testAbortOutdatedRevertCommand();
-    void testIdealStateAbortUpdatesMetric();
-    void testExternalLoadOpAbortUpdatesMetric();
-    void testExternalLoadOpAbortsAreConfigurable();
-    void testAbortCommandsWhenStorageNodeIsDown();
-
-    void setUp() override;
-};
 
-CPPUNIT_TEST_SUITE_REGISTRATION(ChangedBucketOwnershipHandlerTest);
+    void SetUp() override;
+};
 
 document::BucketId
 ChangedBucketOwnershipHandlerTest::nextOwnedBucket(
@@ -171,7 +117,7 @@ ChangedBucketOwnershipHandlerTest::insertBuckets(uint32_t numBuckets,
 }
 
 void
-ChangedBucketOwnershipHandlerTest::setUp()
+ChangedBucketOwnershipHandlerTest::SetUp()
 {
     vdstestlib::DirConfig config(getStandardConfig(true));
 
@@ -185,8 +131,7 @@ ChangedBucketOwnershipHandlerTest::setUp()
     _top->open();
 
     // Ensure we're not dependent on config schema default values.
-    std::unique_ptr<vespa::config::content::PersistenceConfigBuilder> pconfig(
-        new vespa::config::content::PersistenceConfigBuilder);
+    auto pconfig = std::make_unique<vespa::config::content::PersistenceConfigBuilder>();
     pconfig->abortOutdatedMutatingIdealStateOps = true;
     pconfig->abortOutdatedMutatingExternalLoadOps = true;
     _handler->configure(std::move(pconfig));
@@ -253,9 +198,7 @@ ChangedBucketOwnershipHandlerTest::applyClusterState(
     _handler->reloadClusterState();
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testEnumerateBucketsBelongingOnChangedNodes()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, enumerate_buckets_belonging_on_changed_nodes) {
     lib::ClusterState stateBefore("distributor:4 storage:1");
     applyDistribution(Redundancy(1), NodeCount(4));
     applyClusterState(stateBefore);
@@ -267,25 +210,21 @@ ChangedBucketOwnershipHandlerTest::testEnumerateBucketsBelongingOnChangedNodes()
     
     _top->sendDown(createStateCmd("distributor:4 .1.s:d .3.s:d storage:1"));
     // TODO: refactor into own function
-    CPPUNIT_ASSERT_EQUAL(size_t(2), _bottom->getNumCommands());
-    AbortBucketOperationsCommand::SP cmd(
-            std::dynamic_pointer_cast<AbortBucketOperationsCommand>(
-                    _bottom->getCommand(0)));
-    CPPUNIT_ASSERT(cmd.get() != 0);
+    ASSERT_EQ(2, _bottom->getNumCommands());
+    auto cmd = std::dynamic_pointer_cast<AbortBucketOperationsCommand>(_bottom->getCommand(0));
+    ASSERT_TRUE(cmd.get() != 0);
 
-    CPPUNIT_ASSERT(hasAbortedAllOf(cmd, node1Buckets));
-    CPPUNIT_ASSERT(hasAbortedAllOf(cmd, node3Buckets));
-    CPPUNIT_ASSERT(hasAbortedNoneOf(cmd, node0Buckets));
-    CPPUNIT_ASSERT(hasAbortedNoneOf(cmd, node2Buckets));
+    EXPECT_TRUE(hasAbortedAllOf(cmd, node1Buckets));
+    EXPECT_TRUE(hasAbortedAllOf(cmd, node3Buckets));
+    EXPECT_TRUE(hasAbortedNoneOf(cmd, node0Buckets));
+    EXPECT_TRUE(hasAbortedNoneOf(cmd, node2Buckets));
 
     // Handler must swallow abort replies
     _bottom->sendUp(api::StorageMessage::SP(cmd->makeReply().release()));
-    CPPUNIT_ASSERT_EQUAL(size_t(0), _top->getNumReplies());
+    EXPECT_EQ(size_t(0), _top->getNumReplies());
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testNoPreExistingClusterState()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, no_pre_existing_cluster_state) {
     applyDistribution(Redundancy(1), NodeCount(4));
     lib::ClusterState stateBefore("distributor:4 storage:1");
     insertBuckets(2, 1, stateBefore);
@@ -293,7 +232,7 @@ ChangedBucketOwnershipHandlerTest::testNoPreExistingClusterState()
     insertBuckets(2, 2, stateBefore);
     
     _top->sendDown(createStateCmd("distributor:4 .1.s:d .3.s:d storage:1"));
-    CPPUNIT_ASSERT(hasOnlySetSystemStateCmdQueued(*_bottom));
+    EXPECT_TRUE(hasOnlySetSystemStateCmdQueued(*_bottom));
 }
 
 /**
@@ -301,9 +240,7 @@ ChangedBucketOwnershipHandlerTest::testNoPreExistingClusterState()
  * more distributors, we do not send any abort messages since this should
  * already have been done on the down-edge.
  */
-void
-ChangedBucketOwnershipHandlerTest::testNoAvailableDistributorsInCurrentState()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, no_available_distributors_in_current_state) {
     applyDistribution(Redundancy(1), NodeCount(3));
     lib::ClusterState insertedState("distributor:3 storage:1");
     insertBuckets(2, 0, insertedState);
@@ -313,12 +250,10 @@ ChangedBucketOwnershipHandlerTest::testNoAvailableDistributorsInCurrentState()
     _app->setClusterState(downState);
 
     _top->sendDown(createStateCmd("distributor:3 .1.s:d storage:1"));
-    CPPUNIT_ASSERT(hasOnlySetSystemStateCmdQueued(*_bottom));
+    EXPECT_TRUE(hasOnlySetSystemStateCmdQueued(*_bottom));
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testNoAvailableDistributorsInCurrentAndNewState()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, no_available_distributors_in_current_and_new_state) {
     applyDistribution(Redundancy(1), NodeCount(3));
     lib::ClusterState insertedState("distributor:3 storage:1");
     insertBuckets(2, 0, insertedState);
@@ -329,12 +264,10 @@ ChangedBucketOwnershipHandlerTest::testNoAvailableDistributorsInCurrentAndNewSta
     lib::ClusterState downState("distributor:3 .0.s:d .1.s:d .2.s:d storage:1");
 
     _top->sendDown(createStateCmd(downState));
-    CPPUNIT_ASSERT(hasOnlySetSystemStateCmdQueued(*_bottom));
+    EXPECT_TRUE(hasOnlySetSystemStateCmdQueued(*_bottom));
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testDownEdgeToNoAvailableDistributors()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, down_edge_to_no_available_distributors) {
     lib::ClusterState insertedState("distributor:3 storage:1");
     applyDistribution(Redundancy(1), NodeCount(3));
     applyClusterState(insertedState);
@@ -345,20 +278,16 @@ ChangedBucketOwnershipHandlerTest::testDownEdgeToNoAvailableDistributors()
 
     _top->sendDown(createStateCmd(downState));
     // TODO: refactor into own function
-    CPPUNIT_ASSERT_EQUAL(size_t(2), _bottom->getNumCommands());
-    AbortBucketOperationsCommand::SP cmd(
-            std::dynamic_pointer_cast<AbortBucketOperationsCommand>(
-                    _bottom->getCommand(0)));
-    CPPUNIT_ASSERT(cmd.get() != 0);
+    ASSERT_EQ(2, _bottom->getNumCommands());
+    auto cmd = std::dynamic_pointer_cast<AbortBucketOperationsCommand>(_bottom->getCommand(0));
+    ASSERT_TRUE(cmd.get() != 0);
 
-    CPPUNIT_ASSERT(hasAbortedAllOf(cmd, node0Buckets));
-    CPPUNIT_ASSERT(hasAbortedAllOf(cmd, node1Buckets));
-    CPPUNIT_ASSERT(hasAbortedAllOf(cmd, node2Buckets));
+    EXPECT_TRUE(hasAbortedAllOf(cmd, node0Buckets));
+    EXPECT_TRUE(hasAbortedAllOf(cmd, node1Buckets));
+    EXPECT_TRUE(hasAbortedAllOf(cmd, node2Buckets));
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testOwnershipChangedOnDistributorUpEdge()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, ownership_changed_on_distributor_up_edge) {
     lib::ClusterState stateBefore(
             "version:10 distributor:4 .1.s:d storage:4 .1.s:d");
     lib::ClusterState stateAfter(
@@ -373,19 +302,17 @@ ChangedBucketOwnershipHandlerTest::testOwnershipChangedOnDistributorUpEdge()
     
     _top->sendDown(createStateCmd(stateAfter));
     // TODO: refactor into own function
-    CPPUNIT_ASSERT_EQUAL(size_t(2), _bottom->getNumCommands());
-    AbortBucketOperationsCommand::SP cmd(
-            std::dynamic_pointer_cast<AbortBucketOperationsCommand>(
-                    _bottom->getCommand(0)));
-    CPPUNIT_ASSERT(cmd.get() != 0);
+    ASSERT_EQ(2, _bottom->getNumCommands());
+    auto cmd = std::dynamic_pointer_cast<AbortBucketOperationsCommand>(_bottom->getCommand(0));
+    ASSERT_TRUE(cmd.get() != 0);
 
-    CPPUNIT_ASSERT(hasAbortedAllOf(cmd, node1Buckets));
-    CPPUNIT_ASSERT(hasAbortedNoneOf(cmd, node0Buckets));
-    CPPUNIT_ASSERT(hasAbortedNoneOf(cmd, node2Buckets));
+    EXPECT_TRUE(hasAbortedAllOf(cmd, node1Buckets));
+    EXPECT_TRUE(hasAbortedNoneOf(cmd, node0Buckets));
+    EXPECT_TRUE(hasAbortedNoneOf(cmd, node2Buckets));
 
     // Handler must swallow abort replies
     _bottom->sendUp(api::StorageMessage::SP(cmd->makeReply().release()));
-    CPPUNIT_ASSERT_EQUAL(size_t(0), _top->getNumReplies());
+    EXPECT_EQ(0, _top->getNumReplies());
 }
 
 void
@@ -398,21 +325,16 @@ ChangedBucketOwnershipHandlerTest::sendAndExpectAbortedCreateBucket(
 
     _top->sendDown(msg);
     std::vector<api::StorageMessage::SP> replies(_top->getRepliesOnce());
-    CPPUNIT_ASSERT_EQUAL(size_t(1), replies.size());
-    api::StorageReply& reply(dynamic_cast<api::StorageReply&>(*replies[0]));
-    CPPUNIT_ASSERT_EQUAL(api::ReturnCode::ABORTED,
-                         reply.getResult().getResult());
+    ASSERT_EQ(1, replies.size());
+    auto& reply(dynamic_cast<api::StorageReply&>(*replies[0]));
+    EXPECT_EQ(api::ReturnCode::ABORTED, reply.getResult().getResult());
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortOpsWhenNoClusterStateSet()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_ops_when_no_cluster_state_set) {
     sendAndExpectAbortedCreateBucket(1);
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testDistributionConfigChangeUpdatesOwnership()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, distribution_config_change_updates_ownership) {
     lib::ClusterState insertedState("distributor:3 storage:1");
     applyClusterState(insertedState);
     applyDistribution(Redundancy(1), NodeCount(3));
@@ -431,8 +353,8 @@ ChangedBucketOwnershipHandlerTest::testDistributionConfigChangeUpdatesOwnership(
  * owned by distributor 1 in this state to trigger an abort.
  */
 template <typename MsgType, typename... MsgParams>
-bool
-ChangedBucketOwnershipHandlerTest::changeAbortsMessage(MsgParams&&... params)
+void
+ChangedBucketOwnershipHandlerTest::expectChangeAbortsMessage(bool expected, MsgParams&&... params)
 {
     auto msg = std::make_shared<MsgType>(std::forward<MsgParams>(params)...);
     msg->setSourceIndex(1);
@@ -444,15 +366,14 @@ ChangedBucketOwnershipHandlerTest::changeAbortsMessage(MsgParams&&... params)
     std::vector<api::StorageMessage::SP> replies(_top->getRepliesOnce());
     // Test is single-threaded, no need to do any waiting.
     if (replies.empty()) {
-        return false;
+        EXPECT_FALSE(expected);
     } else {
-        CPPUNIT_ASSERT_EQUAL(size_t(1), replies.size());
+        ASSERT_EQ(replies.size(), 1);
         // Make sure the message was actually aborted and not bounced with
         // some other arbitrary failure code.
-        api::StorageReply& reply(dynamic_cast<api::StorageReply&>(*replies[0]));
-        CPPUNIT_ASSERT_EQUAL(api::ReturnCode::ABORTED,
-                             reply.getResult().getResult());
-        return true;
+        auto& reply(dynamic_cast<api::StorageReply&>(*replies[0]));
+        ASSERT_EQ(reply.getResult().getResult(), api::ReturnCode::ABORTED);
+        EXPECT_TRUE(expected);
     }
 }
 
@@ -463,21 +384,21 @@ ChangedBucketOwnershipHandlerTest::changeAbortsMessage(MsgParams&&... params)
  * node is down. This means that any abortable message will trigger an abort.
  */
 template <typename MsgType, typename... MsgParams>
-bool
-ChangedBucketOwnershipHandlerTest::downAbortsMessage(MsgParams&&... params)
+void
+ChangedBucketOwnershipHandlerTest::expectDownAbortsMessage(bool expected, MsgParams&&... params)
 {
     (void) _top->getRepliesOnce();
     (void) _bottom->getCommandsOnce();
-    CPPUNIT_ASSERT((!changeAbortsMessage<MsgType, MsgParams...>(std::forward<MsgParams>(params) ...)));
+    ASSERT_NO_FATAL_FAILURE((expectChangeAbortsMessage<MsgType, MsgParams...>(false, std::forward<MsgParams>(params)...)));
     _top->sendDown(createStateCmd(getStorageDownTestClusterState()));
-    CPPUNIT_ASSERT_EQUAL(size_t(3), _bottom->getNumCommands());
+    ASSERT_EQ(_bottom->getNumCommands(), 3);
     auto setSystemStateCommand = std::dynamic_pointer_cast<api::SetSystemStateCommand>(_bottom->getCommand(2));
-    CPPUNIT_ASSERT(setSystemStateCommand);
+    ASSERT_TRUE(setSystemStateCommand);
     auto abortBucketOperationsCommand = std::dynamic_pointer_cast<AbortBucketOperationsCommand>(_bottom->getCommand(1));
-    CPPUNIT_ASSERT(abortBucketOperationsCommand);
+    ASSERT_TRUE(abortBucketOperationsCommand);
     auto testCommand = _bottom->getCommand(0);
-    CPPUNIT_ASSERT(testCommand);
-    return abortBucketOperationsCommand->shouldAbort(testCommand->getBucket());
+    ASSERT_TRUE(testCommand);
+    EXPECT_EQ(expected, abortBucketOperationsCommand->shouldAbort(testCommand->getBucket()));
 }
 
 /**
@@ -502,59 +423,37 @@ ChangedBucketOwnershipHandlerTest::getBucketToAllow() const
     return makeDocumentBucket(nextOwnedBucket(1, state, document::BucketId()));
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortOutdatedSplit()
-{
-    CPPUNIT_ASSERT(changeAbortsMessage<api::SplitBucketCommand>(
-            getBucketToAbort()));
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::SplitBucketCommand>(
-            getBucketToAllow()));
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_outdated_split) {
+    expectChangeAbortsMessage<api::SplitBucketCommand>(true, getBucketToAbort());
+    expectChangeAbortsMessage<api::SplitBucketCommand>(false, getBucketToAllow());
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortOutdatedJoin()
-{
-    CPPUNIT_ASSERT(changeAbortsMessage<api::JoinBucketsCommand>(
-            getBucketToAbort()));
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::JoinBucketsCommand>(
-            getBucketToAllow()));
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_outdated_join) {
+    expectChangeAbortsMessage<api::JoinBucketsCommand>(true, getBucketToAbort());
+    expectChangeAbortsMessage<api::JoinBucketsCommand>(false, getBucketToAllow());
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortOutdatedSetBucketState()
-{
-    CPPUNIT_ASSERT(changeAbortsMessage<api::SetBucketStateCommand>(
-            getBucketToAbort(), api::SetBucketStateCommand::ACTIVE));
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::SetBucketStateCommand>(
-            getBucketToAllow(), api::SetBucketStateCommand::ACTIVE));
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_outdated_set_bucket_state) {
+    expectChangeAbortsMessage<api::SetBucketStateCommand>(
+            true, getBucketToAbort(), api::SetBucketStateCommand::ACTIVE);
+    expectChangeAbortsMessage<api::SetBucketStateCommand>(
+            false, getBucketToAllow(), api::SetBucketStateCommand::ACTIVE);
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortOutdatedCreateBucket()
-{
-    CPPUNIT_ASSERT(changeAbortsMessage<api::CreateBucketCommand>(
-            getBucketToAbort()));
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::CreateBucketCommand>(
-            getBucketToAllow()));
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_outdated_create_bucket) {
+    expectChangeAbortsMessage<api::CreateBucketCommand>(true, getBucketToAbort());
+    expectChangeAbortsMessage<api::CreateBucketCommand>(false, getBucketToAllow());
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortOutdatedDeleteBucket()
-{
-    CPPUNIT_ASSERT(changeAbortsMessage<api::DeleteBucketCommand>(
-            getBucketToAbort()));
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::DeleteBucketCommand>(
-            getBucketToAllow()));
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_outdated_delete_bucket) {
+    expectChangeAbortsMessage<api::DeleteBucketCommand>(true, getBucketToAbort());
+    expectChangeAbortsMessage<api::DeleteBucketCommand>(false, getBucketToAllow());
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortOutdatedMergeBucket()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_outdated_merge_bucket) {
     std::vector<api::MergeBucketCommand::Node> nodes;
-    CPPUNIT_ASSERT(changeAbortsMessage<api::MergeBucketCommand>(
-            getBucketToAbort(), nodes, 0));
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::MergeBucketCommand>(
-            getBucketToAllow(), nodes, 0));
+    expectChangeAbortsMessage<api::MergeBucketCommand>(true, getBucketToAbort(), nodes, 0);
+    expectChangeAbortsMessage<api::MergeBucketCommand>(false, getBucketToAllow(), nodes, 0);
 }
 
 /**
@@ -562,114 +461,76 @@ ChangedBucketOwnershipHandlerTest::testAbortOutdatedMergeBucket()
  * used as the backing operation for GC we have to treat it as if it were an
  * ideal state operation class.
  */
-void
-ChangedBucketOwnershipHandlerTest::testAbortOutdatedRemoveLocation()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_outdated_remove_location) {
     std::vector<api::MergeBucketCommand::Node> nodes;
-    CPPUNIT_ASSERT(changeAbortsMessage<api::RemoveLocationCommand>(
-            "foo", getBucketToAbort()));
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::RemoveLocationCommand>(
-            "foo", getBucketToAllow()));
+    expectChangeAbortsMessage<api::RemoveLocationCommand>(true, "foo", getBucketToAbort());
+    expectChangeAbortsMessage<api::RemoveLocationCommand>(false, "foo", getBucketToAllow());
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testIdealStateAbortsAreConfigurable()
-{
-    std::unique_ptr<vespa::config::content::PersistenceConfigBuilder> config(
-        new vespa::config::content::PersistenceConfigBuilder);
+TEST_F(ChangedBucketOwnershipHandlerTest, ideal_state_aborts_are_configurable) {
+    auto config = std::make_unique<vespa::config::content::PersistenceConfigBuilder>();
     config->abortOutdatedMutatingIdealStateOps = false;
     _handler->configure(std::move(config));
     // Should not abort operation, even when ownership has changed.
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::CreateBucketCommand>(
-            getBucketToAbort()));
+    expectChangeAbortsMessage<api::CreateBucketCommand>(false, getBucketToAbort());
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortOutdatedPutOperation()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_outdated_put_operation) {
     document::Document::SP doc(_testDocRepo.createRandomDocumentAtLocation(1));
-    CPPUNIT_ASSERT(changeAbortsMessage<api::PutCommand>(
-            getBucketToAbort(), doc, api::Timestamp(1234)));
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::PutCommand>(
-            getBucketToAllow(), doc, api::Timestamp(1234)));
+    expectChangeAbortsMessage<api::PutCommand>(true, getBucketToAbort(), doc, api::Timestamp(1234));
+    expectChangeAbortsMessage<api::PutCommand>(false, getBucketToAllow(), doc, api::Timestamp(1234));
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortOutdatedUpdateCommand()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_outdated_update_command) {
     const document::DocumentType* docType(_testDocRepo.getTypeRepo().getDocumentType("testdoctype1"));
     document::DocumentId docId("id:foo:testdoctype1::bar");
     auto update(std::make_shared<document::DocumentUpdate>(_testDocRepo.getTypeRepo(), *docType, docId));
-    CPPUNIT_ASSERT(changeAbortsMessage<api::UpdateCommand>(getBucketToAbort(), update, api::Timestamp(1234)));
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::UpdateCommand>(getBucketToAllow(), update, api::Timestamp(1234)));
+    expectChangeAbortsMessage<api::UpdateCommand>(true, getBucketToAbort(), update, api::Timestamp(1234));
+    expectChangeAbortsMessage<api::UpdateCommand>(false, getBucketToAllow(), update, api::Timestamp(1234));
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortOutdatedRemoveCommand()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_outdated_remove_command) {
     document::DocumentId docId("id:foo:testdoctype1::bar");
-    CPPUNIT_ASSERT(changeAbortsMessage<api::RemoveCommand>(getBucketToAbort(), docId, api::Timestamp(1234)));
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::RemoveCommand>(getBucketToAllow(), docId, api::Timestamp(1234)));
+    expectChangeAbortsMessage<api::RemoveCommand>(true, getBucketToAbort(), docId, api::Timestamp(1234));
+    expectChangeAbortsMessage<api::RemoveCommand>(false, getBucketToAllow(), docId, api::Timestamp(1234));
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortOutdatedRevertCommand()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_outdated_revert_command) {
     std::vector<api::Timestamp> timestamps;
-    CPPUNIT_ASSERT(changeAbortsMessage<api::RevertCommand>(
-            getBucketToAbort(), timestamps));
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::RevertCommand>(
-            getBucketToAllow(), timestamps));
+    expectChangeAbortsMessage<api::RevertCommand>(true, getBucketToAbort(), timestamps);
+    expectChangeAbortsMessage<api::RevertCommand>(false, getBucketToAllow(), timestamps);
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testIdealStateAbortUpdatesMetric()
-{
-    CPPUNIT_ASSERT(changeAbortsMessage<api::SplitBucketCommand>(
-            getBucketToAbort()));
-    CPPUNIT_ASSERT_EQUAL(
-            uint64_t(1),
-            _handler->getMetrics().idealStateOpsAborted.getValue());
-    CPPUNIT_ASSERT_EQUAL(
-            uint64_t(0),
-            _handler->getMetrics().externalLoadOpsAborted.getValue());
+TEST_F(ChangedBucketOwnershipHandlerTest, ideal_state_abort_updates_metric) {
+    expectChangeAbortsMessage<api::SplitBucketCommand>(true, getBucketToAbort());
+    EXPECT_EQ(1, _handler->getMetrics().idealStateOpsAborted.getValue());
+    EXPECT_EQ(0, _handler->getMetrics().externalLoadOpsAborted.getValue());
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testExternalLoadOpAbortUpdatesMetric()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, external_load_op_abort_updates_metric) {
     document::DocumentId docId("id:foo:testdoctype1::bar");
-    CPPUNIT_ASSERT(changeAbortsMessage<api::RemoveCommand>(
-            getBucketToAbort(), docId, api::Timestamp(1234)));
-    CPPUNIT_ASSERT_EQUAL(
-            uint64_t(0),
-            _handler->getMetrics().idealStateOpsAborted.getValue());
-    CPPUNIT_ASSERT_EQUAL(
-            uint64_t(1),
-            _handler->getMetrics().externalLoadOpsAborted.getValue());
+    expectChangeAbortsMessage<api::RemoveCommand>(
+            true, getBucketToAbort(), docId, api::Timestamp(1234));
+    EXPECT_EQ(0, _handler->getMetrics().idealStateOpsAborted.getValue());
+    EXPECT_EQ(1, _handler->getMetrics().externalLoadOpsAborted.getValue());
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testExternalLoadOpAbortsAreConfigurable()
-{
-    std::unique_ptr<vespa::config::content::PersistenceConfigBuilder> config(
-        new vespa::config::content::PersistenceConfigBuilder);
+TEST_F(ChangedBucketOwnershipHandlerTest, external_load_op_aborts_are_configurable) {
+    auto config = std::make_unique<vespa::config::content::PersistenceConfigBuilder>();
     config->abortOutdatedMutatingExternalLoadOps = false;
     _handler->configure(std::move(config));
     // Should not abort operation, even when ownership has changed.
     document::DocumentId docId("id:foo:testdoctype1::bar");
-    CPPUNIT_ASSERT(!changeAbortsMessage<api::RemoveCommand>(
-            getBucketToAbort(), docId, api::Timestamp(1234)));
+    expectChangeAbortsMessage<api::RemoveCommand>(
+            false, getBucketToAbort(), docId, api::Timestamp(1234));
 }
 
-void
-ChangedBucketOwnershipHandlerTest::testAbortCommandsWhenStorageNodeIsDown()
-{
+TEST_F(ChangedBucketOwnershipHandlerTest, abort_commands_when_storage_node_is_down) {
     document::Document::SP doc(_testDocRepo.createRandomDocumentAtLocation(1));
-    CPPUNIT_ASSERT(downAbortsMessage<api::PutCommand>(
-            getBucketToAllow(), doc, api::Timestamp(1234)));
-    CPPUNIT_ASSERT(downAbortsMessage<api::SetBucketStateCommand>(
-            getBucketToAllow(), api::SetBucketStateCommand::ACTIVE));
+    expectDownAbortsMessage<api::PutCommand>(
+            true, getBucketToAllow(), doc, api::Timestamp(1234));
+    expectDownAbortsMessage<api::SetBucketStateCommand>(
+            true, getBucketToAllow(), api::SetBucketStateCommand::ACTIVE);
 }
 
 } // storage