Simplify and modernize test code.

1 files changed, 77 insertions, 92 deletions

diff --git a/searchcore/src/tests/proton/flushengine/flushengine_test.cpp b/searchcore/src/tests/proton/flushengine/flushengine_test.cpp
index d1a98f1b7d3..9471efb5566 100644
--- a/searchcore/src/tests/proton/flushengine/flushengine_test.cpp
+++ b/searchcore/src/tests/proton/flushengine/flushengine_test.cpp
@@ -3,15 +3,15 @@
 #include <vespa/searchcore/proton/flushengine/cachedflushtarget.h>
 #include <vespa/searchcore/proton/flushengine/flush_engine_explorer.h>
 #include <vespa/searchcore/proton/flushengine/flushengine.h>
+#include <vespa/searchcore/proton/flushengine/i_tls_stats_factory.h>
 #include <vespa/searchcore/proton/flushengine/threadedflushtarget.h>
 #include <vespa/searchcore/proton/flushengine/tls_stats_map.h>
-#include <vespa/searchcore/proton/flushengine/i_tls_stats_factory.h>
 #include <vespa/searchcore/proton/server/igetserialnum.h>
 #include <vespa/searchcore/proton/test/dummy_flush_handler.h>
 #include <vespa/searchcore/proton/test/dummy_flush_target.h>
-#include <vespa/vespalib/testkit/testapp.h>
 #include <vespa/vespalib/data/slime/slime.h>
 #include <vespa/vespalib/test/insertion_operators.h>
+#include <vespa/vespalib/testkit/testapp.h>
 #include <mutex>
 #include <chrono>
 
@@ -42,7 +42,6 @@ public:
     SimpleExecutor()
         : _done()
     {
-        // empty
     }
 
     Task::UP
@@ -113,7 +112,7 @@ public:
     }
 };
 
-typedef std::vector<IFlushTarget::SP> Targets;
+using Targets = std::vector<IFlushTarget::SP>;
 
 using FlushDoneHistory = std::vector<search::SerialNum>;
 
@@ -141,7 +140,6 @@ public:
           _done(targets.size()),
           _flushDoneHistory()
     {
-        // empty
     }
 
     search::SerialNum
@@ -219,7 +217,6 @@ public:
         : _flushedSerial(flushedSerial), _currentSerial(currentSerial),
           _start(start), _done(done), _proceed(proceed)
     {
-        // empty
     }
 
     void run() override {
@@ -270,7 +267,7 @@ public:
         _initDone(),
         _taskStart(),
         _taskDone(),
-        _task(new SimpleTask(_taskStart, _taskDone, &_proceed,
+        _task(std::make_unique<SimpleTask>(_taskStart, _taskDone, &_proceed,
                              _flushedSerial, _currentSerial))
     {
         if (proceedImmediately) {
@@ -366,10 +363,7 @@ public:
 public:
     typedef std::shared_ptr<SimpleStrategy> SP;
 
-    SimpleStrategy()
-    {
-        // empty
-    }
+    SimpleStrategy() {}
 
     uint32_t
     indexOf(const IFlushTarget::SP &target) const
@@ -449,6 +443,14 @@ struct Fixture
     {
     }
 
+    void putFlushHandler(const vespalib::string &docTypeName, IFlushHandler::SP handler) {
+        engine.putFlushHandler(DocTypeName(docTypeName), handler);
+    }
+
+    void addTargetToStrategy(IFlushTarget::SP target) {
+        strategy->_targets.push_back(std::move(target));
+    }
+
     std::shared_ptr<SimpleHandler>
     addSimpleHandler(Targets targets)
     {
@@ -476,16 +478,13 @@ TEST_F("require that strategy controls flush target", Fixture(1, IINTERVAL))
 {
     vespalib::Gate fooG, barG;
     std::vector<vespalib::string> order;
-    FlushTask::UP fooT(new AppendTask("foo", order, fooG));
-    FlushTask::UP barT(new AppendTask("bar", order, barG));
-    SimpleTarget::SP foo(new SimpleTarget(std::move(fooT), "foo"));
-    SimpleTarget::SP bar(new SimpleTarget(std::move(barT), "bar"));
-    f.strategy->_targets.push_back(foo);
-    f.strategy->_targets.push_back(bar);
-
-    SimpleHandler::SP handler(new SimpleHandler({bar, foo}));
-    DocTypeName dtnvanon("anon");
-    f.engine.putFlushHandler(dtnvanon, handler);
+    auto foo = std::make_shared<SimpleTarget>(std::make_unique<AppendTask>("foo", order, fooG), "foo");
+    auto bar = std::make_shared<SimpleTarget>(std::make_unique<AppendTask>("bar", order, barG), "bar");
+    f.addTargetToStrategy(foo);
+    f.addTargetToStrategy(bar);
+
+    auto handler = std::make_shared<SimpleHandler>(Targets({bar, foo}), "anon");
+    f.putFlushHandler("anon", handler);
     f.engine.start();
 
     EXPECT_TRUE(fooG.await(LONG_TIMEOUT));
@@ -502,25 +501,20 @@ TEST_F("require that zero handlers does not core", Fixture(2, 50))
 
 TEST_F("require that zero targets does not core", Fixture(2, 50))
 {
-    DocTypeName dtnvfoo("foo");
-    DocTypeName dtnvbar("bar");
-    f.engine.putFlushHandler(dtnvfoo,
-                             IFlushHandler::SP(new SimpleHandler({}, "foo")));
-    f.engine.putFlushHandler(dtnvbar,
-                             IFlushHandler::SP(new SimpleHandler({}, "bar")));
+    f.putFlushHandler("foo", std::make_shared<SimpleHandler>(Targets(), "foo"));
+    f.putFlushHandler("bar", std::make_shared<SimpleHandler>(Targets(), "bar"));
     f.engine.start();
 }
 
 TEST_F("require that oldest serial is found", Fixture(1, IINTERVAL))
 {
-    SimpleTarget::SP foo(new SimpleTarget("foo", 10));
-    SimpleTarget::SP bar(new SimpleTarget("bar", 20));
-    f.strategy->_targets.push_back(foo);
-    f.strategy->_targets.push_back(bar);
-
-    SimpleHandler::SP handler(new SimpleHandler({foo, bar}, "anon", 25));
-    DocTypeName dtnvanon("anon");
-    f.engine.putFlushHandler(dtnvanon, handler);
+    auto foo = std::make_shared<SimpleTarget>("foo", 10);
+    auto bar = std::make_shared<SimpleTarget>("bar", 20);
+    f.addTargetToStrategy(foo);
+    f.addTargetToStrategy(bar);
+
+    auto handler = std::make_shared<SimpleHandler>(Targets({foo, bar}), "anon", 25);
+    f.putFlushHandler("anon", handler);
     f.engine.start();
 
     EXPECT_TRUE(handler->_done.await(LONG_TIMEOUT));
@@ -531,22 +525,20 @@ TEST_F("require that oldest serial is found", Fixture(1, IINTERVAL))
 
 TEST_F("require that oldest serial is found in group", Fixture(2, IINTERVAL))
 {
-    SimpleTarget::SP fooT1(new SimpleTarget("fooT1", 10));
-    SimpleTarget::SP fooT2(new SimpleTarget("fooT2", 20));
-    SimpleTarget::SP barT1(new SimpleTarget("barT1",  5));
-    SimpleTarget::SP barT2(new SimpleTarget("barT2", 15));
-    f.strategy->_targets.push_back(fooT1);
-    f.strategy->_targets.push_back(fooT2);
-    f.strategy->_targets.push_back(barT1);
-    f.strategy->_targets.push_back(barT2);
-
-    SimpleHandler::SP fooH(new SimpleHandler({fooT1, fooT2}, "fooH", 25));
-    DocTypeName dtnvfoo("foo");
-    f.engine.putFlushHandler(dtnvfoo, fooH);
-
-    SimpleHandler::SP barH(new SimpleHandler({barT1, barT2}, "barH", 20));
-    DocTypeName dtnvbar("bar");
-    f.engine.putFlushHandler(dtnvbar, barH);
+    auto fooT1 = std::make_shared<SimpleTarget>("fooT1", 10);
+    auto fooT2 = std::make_shared<SimpleTarget>("fooT2", 20);
+    auto barT1 = std::make_shared<SimpleTarget>("barT1",  5);
+    auto barT2 = std::make_shared<SimpleTarget>("barT2", 15);
+    f.addTargetToStrategy(fooT1);
+    f.addTargetToStrategy(fooT2);
+    f.addTargetToStrategy(barT1);
+    f.addTargetToStrategy(barT2);
+
+    auto fooH = std::make_shared<SimpleHandler>(Targets({fooT1, fooT2}), "fooH", 25);
+    f.putFlushHandler("foo", fooH);
+
+    auto barH = std::make_shared<SimpleHandler>(Targets({barT1, barT2}), "barH", 20);
+    f.putFlushHandler("bar", barH);
 
     f.engine.start();
 
@@ -574,11 +566,10 @@ TEST_F("require that oldest serial is found in group", Fixture(2, IINTERVAL))
 
 TEST_F("require that target can refuse flush", Fixture(2, IINTERVAL))
 {
-    SimpleTarget::SP target(new SimpleTarget());
-    SimpleHandler::SP handler(new SimpleHandler({target}));
+    auto target = std::make_shared<SimpleTarget>();
+    auto handler = std::make_shared<SimpleHandler>(Targets({target}));
     target->_task = searchcorespi::FlushTask::UP();
-    DocTypeName dtnvanon("anon");
-    f.engine.putFlushHandler(dtnvanon, handler);
+    f.putFlushHandler("anon", handler);
     f.engine.start();
 
     EXPECT_TRUE(target->_initDone.await(LONG_TIMEOUT));
@@ -589,10 +580,9 @@ TEST_F("require that target can refuse flush", Fixture(2, IINTERVAL))
 TEST_F("require that targets are flushed when nothing new to flush",
        Fixture(2, IINTERVAL))
 {
-    SimpleTarget::SP target(new SimpleTarget("anon", 5)); // oldest unflushed serial num = 5
-    SimpleHandler::SP handler(new SimpleHandler({target}, "anon", 4)); // current serial num = 4
-    DocTypeName dtnvanon("anon");
-    f.engine.putFlushHandler(dtnvanon, handler);
+    auto target = std::make_shared<SimpleTarget>("anon", 5); // oldest unflushed serial num = 5
+    auto handler = std::make_shared<SimpleHandler>(Targets({target}), "anon", 4); // current serial num = 4
+    f.putFlushHandler("anon", handler);
     f.engine.start();
 
     EXPECT_TRUE(target->_initDone.await(LONG_TIMEOUT));
@@ -602,14 +592,13 @@ TEST_F("require that targets are flushed when nothing new to flush",
 
 TEST_F("require that flushing targets are skipped", Fixture(2, IINTERVAL))
 {
-    SimpleTarget::SP foo(new SimpleTarget("foo"));
-    SimpleTarget::SP bar(new SimpleTarget("bar"));
-    f.strategy->_targets.push_back(foo);
-    f.strategy->_targets.push_back(bar);
-
-    SimpleHandler::SP handler(new SimpleHandler({bar, foo}));
-    DocTypeName dtnvanon("anon");
-    f.engine.putFlushHandler(dtnvanon, handler);
+    auto foo = std::make_shared<SimpleTarget>("foo");
+    auto bar = std::make_shared<SimpleTarget>("bar");
+    f.addTargetToStrategy(foo);
+    f.addTargetToStrategy(bar);
+
+    auto handler = std::make_shared<SimpleHandler>(Targets({bar, foo}));
+    f.putFlushHandler("anon", handler);
     f.engine.start();
 
     EXPECT_TRUE(foo->_taskDone.await(LONG_TIMEOUT));
@@ -618,12 +607,11 @@ TEST_F("require that flushing targets are skipped", Fixture(2, IINTERVAL))
 
 TEST_F("require that updated targets are not skipped", Fixture(2, IINTERVAL))
 {
-    SimpleTarget::SP target(new SimpleTarget("target", 1));
-    f.strategy->_targets.push_back(target);
+    auto target = std::make_shared<SimpleTarget>("target", 1);
+    f.addTargetToStrategy(target);
 
-    SimpleHandler::SP handler(new SimpleHandler({target}, "handler", 0));
-    DocTypeName dtnvhandler("handler");
-    f.engine.putFlushHandler(dtnvhandler, handler);
+    auto handler = std::make_shared<SimpleHandler>(Targets({target}), "handler", 0);
+    f.putFlushHandler("handler", handler);
     f.engine.start();
 
     EXPECT_TRUE(target->_taskDone.await(LONG_TIMEOUT));
@@ -633,8 +621,7 @@ TEST("require that threaded target works")
 {
     SimpleExecutor executor;
     SimpleGetSerialNum getSerialNum;
-    IFlushTarget::SP target(new SimpleTarget());
-    target.reset(new ThreadedFlushTarget(executor, getSerialNum, target));
+    auto target = std::make_shared<ThreadedFlushTarget>(executor, getSerialNum, std::make_shared<SimpleTarget>());
 
     EXPECT_FALSE(executor._done.await(SHORT_TIMEOUT));
     EXPECT_TRUE(target->initFlush(0).get() != NULL);
@@ -643,8 +630,7 @@ TEST("require that threaded target works")
 
 TEST("require that cached target works")
 {
-    IFlushTarget::SP target(new AssertedTarget());
-    target.reset(new CachedFlushTarget(target));
+    auto target = std::make_shared<CachedFlushTarget>(std::make_shared<AssertedTarget>());
     for (uint32_t i = 0; i < 2; ++i) {
         EXPECT_EQUAL(0l, target->getApproxMemoryGain().getBefore());
         EXPECT_EQUAL(0l, target->getApproxMemoryGain().getAfter());
@@ -654,12 +640,11 @@ TEST("require that cached target works")
 
 TEST_F("require that trigger flush works", Fixture(2, IINTERVAL))
 {
-    SimpleTarget::SP target(new SimpleTarget("target", 1));
-    f.strategy->_targets.push_back(target);
+    auto target = std::make_shared<SimpleTarget>("target", 1);
+    f.addTargetToStrategy(target);
 
-    SimpleHandler::SP handler(new SimpleHandler({target}, "handler", 9));
-    DocTypeName dtnvhandler("handler");
-    f.engine.putFlushHandler(dtnvhandler, handler);
+    auto handler = std::make_shared<SimpleHandler>(Targets({target}), "handler", 9);
+    f.putFlushHandler("handler", handler);
     f.engine.start();
     f.engine.triggerFlush();
     EXPECT_TRUE(target->_initDone.await(LONG_TIMEOUT));
@@ -693,13 +678,13 @@ assertThatHandlersInCurrentSet(FlushEngine & engine, const std::vector<const cha
 
 TEST_F("require that concurrency works", Fixture(2, 1))
 {
-    SimpleTarget::SP target1(new SimpleTarget("target1", 1, false));
-    SimpleTarget::SP target2(new SimpleTarget("target2", 2, false));
-    SimpleTarget::SP target3(new SimpleTarget("target3", 3, false));
-    SimpleHandler::SP handler(new SimpleHandler({target1, target2, target3}, "handler", 9));
-    DocTypeName dtnvhandler("handler");
-    f.engine.putFlushHandler(dtnvhandler, handler);
+    auto target1 = std::make_shared<SimpleTarget>("target1", 1, false);
+    auto target2 = std::make_shared<SimpleTarget>("target2", 2, false);
+    auto target3 = std::make_shared<SimpleTarget>("target3", 3, false);
+    auto handler = std::make_shared<SimpleHandler>(Targets({target1, target2, target3}), "handler", 9);
+    f.putFlushHandler("handler", handler);
     f.engine.start();
+
     EXPECT_TRUE(target1->_initDone.await(LONG_TIMEOUT));
     EXPECT_TRUE(target2->_initDone.await(LONG_TIMEOUT));
     EXPECT_TRUE(!target3->_initDone.await(SHORT_TIMEOUT));
@@ -714,11 +699,11 @@ TEST_F("require that concurrency works", Fixture(2, 1))
 
 TEST_F("require that state explorer can list flush targets", Fixture(1, 1))
 {
-    SimpleTarget::SP target = std::make_shared<SimpleTarget>("target1", 100, false);
-    f.engine.putFlushHandler(DocTypeName("handler"),
-            std::make_shared<SimpleHandler>(
-                    Targets({target, std::make_shared<SimpleTarget>("target2", 50, true)}),
-                             "handler", 9));
+    auto target = std::make_shared<SimpleTarget>("target1", 100, false);
+    f.putFlushHandler("handler",
+                      std::make_shared<SimpleHandler>(
+                              Targets({target, std::make_shared<SimpleTarget>("target2", 50, true)}),
+                              "handler", 9));
     f.engine.start();
     target->_initDone.await(LONG_TIMEOUT);
     target->_taskStart.await(LONG_TIMEOUT);