Fixing agency prefix in Agency/Job.cpp (#5039)

* Fixing some test issues and fixing the agency prefix in Agency/Job.cpp
* Making logic consistent in  failed- leader / follower job
* reverting condition back to == GOOD

arangod/Agency/AddFollower.cpp

@@ -1,7 +1,7 @@
 ////////////////////////////////////////////////////////////////////////////////
 /// DISCLAIMER
 ///
-/// Copyright 2016 ArangoDB GmbH, Cologne, Germany
+/// Copyright 2018 ArangoDB GmbH, Cologne, Germany
 ///
 /// Licensed under the Apache License, Version 2.0 (the "License");
 /// you may not use this file except in compliance with the License.
@@ -82,9 +82,7 @@ bool AddFollower::create(std::shared_ptr<VPackBuilder> envelope) {
     _jb->openObject();
   }
 
-  std::string path = toDoPrefix + _jobId;
+  _jb->add(VPackValue(toDoPrefix + _jobId));
-
-  _jb->add(VPackValue(path));
   { VPackObjectBuilder guard(_jb.get());
     _jb->add("creator", VPackValue(_creator));
     _jb->add("type", VPackValue("addFollower"));
@@ -166,7 +164,7 @@ bool AddFollower::start() {
   // Remove those that are not in state "GOOD":
   auto it = available.begin();
   while (it != available.end()) {
-    if (checkServerGood(_snapshot, *it) != "GOOD") {
+    if (checkServerHealth(_snapshot, *it) != "GOOD") {
       it = available.erase(it);
     } else {
       ++it;
@@ -258,7 +256,7 @@ bool AddFollower::start() {
       addPreconditionUnchanged(trx, planPath, planned);
       addPreconditionShardNotBlocked(trx, _shard);
       for (auto const& srv : chosen) {
-        addPreconditionServerGood(trx, srv);
+        addPreconditionServerHealth(trx, srv, "GOOD");
       }
     }   // precondition done
   }  // array for transaction done
@@ -269,7 +267,7 @@ bool AddFollower::start() {
   if (res.accepted && res.indices.size() == 1 && res.indices[0]) {
     _status = FINISHED;
     LOG_TOPIC(INFO, Logger::SUPERVISION)
-      << "Pending: Addfollower(s) to shard " << _shard << " in collection "
+      << "Finished: Addfollower(s) to shard " << _shard << " in collection "
       << _collection;
     return true;
   }
@@ -307,4 +305,3 @@ arangodb::Result AddFollower::abort() {
   return result;
   
 }
-

arangod/Agency/AgencyFeature.cpp

@@ -247,8 +247,7 @@ void AgencyFeature::start() {
   if (!feature->agencyPrefix().empty()) {
     arangodb::consensus::Supervision::setAgencyPrefix(
       std::string("/") + feature->agencyPrefix());
-    arangodb::consensus::Job::agencyPrefix
+    arangodb::consensus::Job::agencyPrefix = feature->agencyPrefix();;
-      = std::string("/") + feature->agencyPrefix();
   }
   
   // TODO: Port this to new options handling

arangod/Agency/Agent.h

@@ -42,8 +42,8 @@ struct TRI_vocbase_t;
 namespace arangodb {
 namespace consensus {
 
-class Agent : public arangodb::Thread,
+class Agent final : public arangodb::Thread,
-              public AgentInterface {
+                    public AgentInterface {
 
  public:
   /// @brief Construct with program options

arangod/Agency/CleanOutServer.cpp

@@ -206,7 +206,7 @@ bool CleanOutServer::start() {
   }
 
   // Check that the server is in state "GOOD":
-  std::string health = checkServerGood(_snapshot, _server);
+  std::string health = checkServerHealth(_snapshot, _server);
   if (health != "GOOD") {
     LOG_TOPIC(DEBUG, Logger::SUPERVISION) << "server " << _server
       << " is currently " << health << ", not starting CleanOutServer job "
@@ -317,7 +317,7 @@ bool CleanOutServer::start() {
     // Preconditions
     { VPackObjectBuilder objectForPrecondition(pending.get());
       addPreconditionServerNotBlocked(*pending, _server);
-      addPreconditionServerGood(*pending, _server);
+      addPreconditionServerHealth(*pending, _server, "GOOD");
       addPreconditionUnchanged(*pending, failedServersPrefix, failedServers);
       addPreconditionUnchanged(*pending, cleanedPrefix, cleanedServers);
     }

arangod/Agency/FailedFollower.cpp

@@ -211,7 +211,7 @@ bool FailedFollower::start() {
         // shard not blocked
         addPreconditionShardNotBlocked(job, _shard);
         // toServer in good condition 
-        addPreconditionServerGood(job, _to);
+        addPreconditionServerHealth(job, _to, "GOOD");
       } 
         
     }
@@ -245,8 +245,8 @@ bool FailedFollower::start() {
 
   auto slice = result.get(
     std::vector<std::string>(
-      {agencyPrefix, "Supervision", "Health", _from, "Status"}));
+      { agencyPrefix, "Supervision", "Health", _from, "Status"}));
-  if (!slice.isString() || slice.copyString() != "FAILED") {
+  if (slice.isString() && slice.copyString() != "FAILED") {
     finish("", _shard, false, "Server " + _from + " no longer failing.");
   }
 

arangod/Agency/FailedLeader.cpp

@@ -264,11 +264,9 @@ bool FailedLeader::start() {
       // Preconditions -------------------------------------------------------
       { VPackObjectBuilder preconditions(&pending);
         // Failed condition persists
-        pending.add(VPackValue(healthPrefix + _from + "/Status"));
+        addPreconditionServerHealth(pending, _from, "FAILED");
-        { VPackObjectBuilder stillExists(&pending);
-          pending.add("old", VPackValue("FAILED")); }
         // Destination server still in good condition
-        addPreconditionServerGood(pending, _to);
+        addPreconditionServerHealth(pending, _to, "GOOD"); 
         // Server list in plan still as before
         addPreconditionUnchanged(pending, planPath, planned);
         // Destination server should not be blocked by another job

arangod/Agency/FailedServer.cpp

@@ -116,23 +116,16 @@ bool FailedServer::start() {
         }
       }
       // Delete todo
-      pending.add(VPackValue(toDoPrefix + _jobId));
+      addRemoveJobFromSomewhere(pending, "ToDo", _jobId);
-      { VPackObjectBuilder del(&pending);
-        pending.add("op", VPackValue("delete")); }
-
       addBlockServer(pending, _server, _jobId);
     } // <------------ Operations
     
     // Preconditions ----------->
     { VPackObjectBuilder prec(&pending);
       // Check that toServer not blocked
-      pending.add(VPackValue(blockedServersPrefix + _server));
+      addPreconditionServerNotBlocked(pending, _server);
-      { VPackObjectBuilder block(&pending);
-        pending.add("oldEmpty", VPackValue(true)); }
       // Status should still be FAILED
-      pending.add(VPackValue(healthPrefix + _server + "/Status"));
+      addPreconditionServerHealth(pending, _server, "FAILED");
-      { VPackObjectBuilder old(&pending);
-        pending.add("old", VPackValue("FAILED")); }
     } // <--------- Preconditions
   }
 
@@ -237,9 +230,7 @@ bool FailedServer::create(std::shared_ptr<VPackBuilder> envelope) {
     //Preconditions
     { VPackObjectBuilder health(_jb.get());
       // Status should still be BAD
-      _jb->add(VPackValue(healthPrefix + _server + "/Status"));
+      addPreconditionServerHealth(*_jb, _server, "BAD");
-      { VPackObjectBuilder old(_jb.get());
-        _jb->add("old", VPackValue("BAD")); }
       // Target/FailedServers does not already include _server
       _jb->add(VPackValue(failedServersPrefix + "/" + _server));
       { VPackObjectBuilder old(_jb.get());

arangod/Agency/Job.cpp

@@ -67,7 +67,7 @@ Job::Job(JOB_STATUS status, Node const& snapshot, AgentInterface* agent,
 Job::~Job() {}
 
 // this will be initialized in the AgencyFeature
-std::string Job::agencyPrefix = "/arango";
+std::string Job::agencyPrefix = "arango";
 
 bool Job::finish(
   std::string const& server, std::string const& shard,
@@ -206,7 +206,7 @@ std::string Job::randomIdleGoodAvailableServer(Node const& snap,
 
 }
 
-
+/// @brief Get servers from plan, which are not failed or cleaned out
 std::vector<std::string> Job::availableServers(Node const& snapshot) {
 
   std::vector<std::string> ret;
@@ -217,7 +217,7 @@ std::vector<std::string> Job::availableServers(Node const& snapshot) {
     ret.push_back(srv.first);
   }
 
-  // Remove cleaned servers from ist
+  // Remove cleaned servers from list
   try {
     for (auto const& srv :
            VPackArrayIterator(snapshot(cleanedPrefix).slice())) {
@@ -500,11 +500,12 @@ void Job::addPreconditionServerNotBlocked(Builder& pre, std::string const& serve
 	}
 }
 
-void Job::addPreconditionServerGood(Builder& pre, std::string const& server) {
+void Job::addPreconditionServerHealth(Builder& pre, std::string const& server,
-	pre.add(VPackValue(healthPrefix + server + "/Status"));
+                                      std::string const& health) {
-	{ VPackObjectBuilder serverGood(&pre);
+  pre.add(VPackValue(healthPrefix + server + "/Status"));
-		pre.add("old", VPackValue("GOOD"));
+  { VPackObjectBuilder serverGood(&pre);
-	}
+    pre.add("old", VPackValue(health));
+  }
 }
 
 void Job::addPreconditionShardNotBlocked(Builder& pre, std::string const& shard) {
@@ -544,13 +545,10 @@ void Job::addReleaseShard(Builder& trx, std::string const& shard) {
   }
 }
 
-std::string Job::checkServerGood(Node const& snapshot,
+std::string Job::checkServerHealth(Node const& snapshot,
-                                 std::string const& server) {
+                                   std::string const& server) {
   if (!snapshot.has(healthPrefix + server + "/Status")) {
     return "UNCLEAR";
   }
-  if (snapshot(healthPrefix + server + "/Status").getString() != "GOOD") {
+  return snapshot(healthPrefix + server + "/Status").getString();
-    return "UNHEALTHY";
-  }
-  return "GOOD";
 }

arangod/Agency/Job.h

@@ -58,16 +58,6 @@ extern std::string const planVersion;
 extern std::string const plannedServers;
 extern std::string const healthPrefix;
 
-struct JobResult {
-  JobResult() {}
-};
-
-struct JobCallback {
-  JobCallback() {}
-  virtual ~JobCallback(){};
-  virtual bool operator()(JobResult*) = 0;
-};
-
 struct Job {
 
   struct shard_t {
@@ -135,6 +125,7 @@ struct Job {
   static std::string randomIdleGoodAvailableServer(
     Node const& snap, VPackSlice const& exclude);
   
+  /// @brief Get servers from plan, which are not failed or cleaned out
   static std::vector<std::string> availableServers(
     const arangodb::consensus::Node&);
 
@@ -151,6 +142,7 @@ struct Job {
   AgentInterface* _agent;
   std::string _jobId;
   std::string _creator;
+  
   static std::string agencyPrefix;  // will be initialized in AgencyFeature
 
   std::shared_ptr<Builder> _jb;
@@ -177,12 +169,11 @@ struct Job {
   static void addReleaseServer(Builder& trx, std::string const& server);
   static void addReleaseShard(Builder& trx, std::string const& shard);
   static void addPreconditionServerNotBlocked(Builder& pre, std::string const& server);
-  static void addPreconditionServerGood(Builder& pre, std::string const& server);
+  static void addPreconditionServerHealth(Builder& pre, std::string const& server, std::string const& health);
   static void addPreconditionShardNotBlocked(Builder& pre, std::string const& shard);
   static void addPreconditionUnchanged(Builder& pre,
     std::string const& key, Slice value);
-  static std::string checkServerGood(Node const& snapshot,
+  static std::string checkServerHealth(Node const& snapshot, std::string const& server);
-                                     std::string const& server);
 
 };
 
@@ -199,13 +190,13 @@ inline arangodb::consensus::write_ret_t singleWriteTransaction(
       VPackArrayBuilder onePair(envelope.get());
       { VPackObjectBuilder mutationPart(envelope.get());
         for (auto const& pair : VPackObjectIterator(trx[0])) {
-          envelope->add(Job::agencyPrefix + pair.key.copyString(), pair.value);
+          envelope->add("/" + Job::agencyPrefix + pair.key.copyString(), pair.value);
         }
       }
       if (trx.length() > 1) {
         VPackObjectBuilder preconditionPart(envelope.get());
         for (auto const& pair : VPackObjectIterator(trx[1])) {
-          envelope->add(Job::agencyPrefix + pair.key.copyString(), pair.value);
+          envelope->add("/" + Job::agencyPrefix + pair.key.copyString(), pair.value);
         }
       }
     }
@@ -239,19 +230,19 @@ inline arangodb::consensus::trans_ret_t generalTransaction(
           VPackArrayBuilder onePair(envelope.get());
           { VPackObjectBuilder mutationPart(envelope.get());
             for (auto const& pair : VPackObjectIterator(singleTrans[0])) {
-              envelope->add(Job::agencyPrefix + pair.key.copyString(), pair.value);
+              envelope->add("/" + Job::agencyPrefix + pair.key.copyString(), pair.value);
             }
           }
           if (singleTrans.length() > 1) {
             VPackObjectBuilder preconditionPart(envelope.get());
             for (auto const& pair : VPackObjectIterator(singleTrans[1])) {
-              envelope->add(Job::agencyPrefix + pair.key.copyString(), pair.value);
+              envelope->add("/" + Job::agencyPrefix + pair.key.copyString(), pair.value);
             }
           }
         } else if (singleTrans[0].isString()) {
           VPackArrayBuilder reads(envelope.get());
           for (auto const& path : VPackArrayIterator(singleTrans)) {
-            envelope->add(VPackValue(Job::agencyPrefix + path.copyString()));
+            envelope->add(VPackValue("/" + Job::agencyPrefix + path.copyString()));
           }
         }
       }
@@ -282,13 +273,13 @@ inline arangodb::consensus::trans_ret_t transient(AgentInterface* _agent,
       VPackArrayBuilder onePair(envelope.get());
       { VPackObjectBuilder mutationPart(envelope.get());
         for (auto const& pair : VPackObjectIterator(trx[0])) {
-          envelope->add(Job::agencyPrefix + pair.key.copyString(), pair.value);
+          envelope->add("/" + Job::agencyPrefix + pair.key.copyString(), pair.value);
         }
       }
       if (trx.length() > 1) {
         VPackObjectBuilder preconditionPart(envelope.get());
         for (auto const& pair : VPackObjectIterator(trx[1])) {
-          envelope->add(Job::agencyPrefix + pair.key.copyString(), pair.value);
+          envelope->add("/" + Job::agencyPrefix + pair.key.copyString(), pair.value);
         }
       }
     }

arangod/Agency/JobContext.cpp

@@ -41,36 +41,22 @@ JobContext::JobContext (JOB_STATUS status, std::string id, Node const& snapshot,
   auto const& type = job("type").getString();
   
   if        (type == "failedLeader") {
-    _job =
+    _job = std::make_unique<FailedLeader>(snapshot, agent, status, id);
-      std::unique_ptr<FailedLeader>(
-        new FailedLeader(snapshot, agent, status, id));
   } else if (type == "failedFollower") {
-    _job =
+    _job = std::make_unique<FailedFollower>(snapshot, agent, status, id);
-      std::unique_ptr<FailedFollower>(
-        new FailedFollower(snapshot, agent, status, id));
   } else if (type == "failedServer") {
-    _job =
+    _job = std::make_unique<FailedServer>(snapshot, agent, status, id);
-      std::unique_ptr<FailedServer>(
-        new FailedServer(snapshot, agent, status, id));
   } else if (type == "cleanOutServer") {
-    _job =
+    _job = std::make_unique<CleanOutServer>(snapshot, agent, status, id);
-      std::unique_ptr<CleanOutServer>(
-        new CleanOutServer(snapshot, agent, status, id));
   } else if (type == "moveShard") {
-    _job =
+    _job = std::make_unique<MoveShard>(snapshot, agent, status, id);
-      std::unique_ptr<MoveShard>(
-        new MoveShard(snapshot, agent, status, id));
   } else if (type == "addFollower") {
-    _job =
+    _job = std::make_unique<AddFollower>(snapshot, agent, status, id);
-      std::unique_ptr<AddFollower>(
-        new AddFollower(snapshot, agent, status, id));
   } else if (type == "removeFollower") {
-    _job =
+    _job = std::make_unique<RemoveFollower>(snapshot, agent, status, id);
-      std::unique_ptr<RemoveFollower>(
-        new RemoveFollower(snapshot, agent, status, id));
   } else {
     LOG_TOPIC(ERR, Logger::AGENCY) <<
-      "Failed to run supervision job " << type << " with id " << id;
+    "Failed to run supervision job " << type << " with id " << id;
   }
 
 }

arangod/Agency/MoveShard.cpp

@@ -187,7 +187,7 @@ bool MoveShard::start() {
   }
 
   // Check that the toServer is in state "GOOD":
-  std::string health = checkServerGood(_snapshot, _to);
+  std::string health = checkServerHealth(_snapshot, _to);
   if (health != "GOOD") {
     LOG_TOPIC(DEBUG, Logger::SUPERVISION) << "server " << _to
       << " is currently " << health << ", not starting MoveShard job "
@@ -348,7 +348,7 @@ bool MoveShard::start() {
       addPreconditionUnchanged(pending, planPath, planned);
       addPreconditionShardNotBlocked(pending, _shard);
       addPreconditionServerNotBlocked(pending, _to);
-      addPreconditionServerGood(pending, _to);
+      addPreconditionServerHealth(pending, _to, "GOOD"); 
       addPreconditionUnchanged(pending, failedServersPrefix, failedServers);
       addPreconditionUnchanged(pending, cleanedPrefix, cleanedServers);
     }   // precondition done

arangod/Agency/RemoveFollower.cpp

@@ -169,7 +169,7 @@ bool RemoveFollower::start() {
   bool leaderBad = false;
   for (auto const& srv : VPackArrayIterator(planned)) {
     std::string serverName = srv.copyString();
-    if (checkServerGood(_snapshot, serverName) == "GOOD") {
+    if (checkServerHealth(_snapshot, serverName) == "GOOD") {
       overview.emplace(serverName, 0);
     } else {
       overview.emplace(serverName, -1);
@@ -357,7 +357,7 @@ bool RemoveFollower::start() {
       addPreconditionUnchanged(trx, planPath, planned);
       addPreconditionShardNotBlocked(trx, _shard);
       for (auto const& srv : kept) {
-        addPreconditionServerGood(trx, srv);
+        addPreconditionServerHealth(trx, srv, "GOOD");
       }
     }   // precondition done
   }  // array for transaction done

tests/Agency/AddFollowerTest.cpp

@@ -28,8 +28,6 @@
 #include "fakeit.hpp"
 
 #include "Agency/AddFollower.h"
-#include "Agency/FailedLeader.h"
-#include "Agency/MoveShard.h"
 #include "Agency/AgentInterface.h"
 #include "Agency/Node.h"
 #include "lib/Basics/StringUtils.h"
@@ -151,7 +149,7 @@ TEST_CASE("AddFollower", "[agency][supervision]") {
     When(Method(mockAgent, waitFor)).AlwaysReturn(AgentInterface::raft_commit_t::OK);
     auto& agent = mockAgent.get();
     auto  addFollower = AddFollower(
-      baseStructure, &agent, jobId, "unittest", DATABASE, COLLECTION, SHARD);
+      baseStructure(PREFIX), &agent, jobId, "unittest", DATABASE, COLLECTION, SHARD);
     
     addFollower.create();
     
@@ -180,7 +178,6 @@ TEST_CASE("AddFollower", "[agency][supervision]") {
         if (path == "/arango/Target/ToDo") {
           builder->add(jobId, createBuilder(todo).slice());
         }
-        builder->close();
       } else {
         builder->add(s);
       }

tests/Agency/FailedFollowerTest.cpp

@@ -230,7 +230,7 @@ SECTION("if we want to start and the collection went missing from plan (our trut
   When(Method(mockAgent, waitFor)).AlwaysReturn(AgentInterface::raft_commit_t::OK);
   AgentInterface &agent = mockAgent.get();
   auto failedFollower = FailedFollower(
-    agency("arango"),
+    agency(PREFIX),
     &agent,
     JOB_STATUS::TODO,
     jobId
@@ -290,7 +290,7 @@ SECTION("if we are supposed to fail a distributeShardsLike job we immediately fa
   When(Method(mockAgent, waitFor)).AlwaysReturn(AgentInterface::raft_commit_t::OK);
   AgentInterface &agent = mockAgent.get();
   auto failedFollower = FailedFollower(
-    agency("arango"),
+    agency(PREFIX),
     &agent,
     JOB_STATUS::TODO,
     jobId
@@ -352,12 +352,14 @@ SECTION("if the follower is healthy again we fail the job") {
   When(Method(mockAgent, waitFor)).AlwaysReturn();
   AgentInterface &agent = mockAgent.get();
   auto failedFollower = FailedFollower(
-    agency("arango"),
+    agency(PREFIX),
     &agent,
     JOB_STATUS::TODO,
     jobId
   );
-  failedFollower.start();
+  REQUIRE_FALSE(failedFollower.start());
+  Verify(Method(mockAgent, transact));
+  Verify(Method(mockAgent, write));
 }
 
 SECTION("if there is no healthy free server when trying to start just wait") {
@@ -400,12 +402,12 @@ SECTION("if there is no healthy free server when trying to start just wait") {
   Mock<AgentInterface> mockAgent;
   AgentInterface &agent = mockAgent.get();
   auto failedFollower = FailedFollower(
-    agency("arango"),
+    agency(PREFIX),
     &agent,
     JOB_STATUS::TODO,
     jobId
   );
-  failedFollower.start();
+  REQUIRE_FALSE(failedFollower.start());
 }
 
 SECTION("abort any moveShard job blocking the shard and start") {
@@ -427,7 +429,7 @@ SECTION("abort any moveShard job blocking the shard and start") {
   When(Method(moveShardMockAgent, waitFor)).Return();
   AgentInterface &moveShardAgent = moveShardMockAgent.get();
   auto moveShard = MoveShard(
-    baseStructure("arango"), &moveShardAgent, "2", "strunz", DATABASE,
+    baseStructure(PREFIX), &moveShardAgent, "2", "strunz", DATABASE,
     COLLECTION, SHARD, SHARD_LEADER, FREE_SERVER, true);
   moveShard.create();
 
@@ -489,12 +491,12 @@ SECTION("abort any moveShard job blocking the shard and start") {
   When(Method(mockAgent, waitFor)).AlwaysReturn(AgentInterface::raft_commit_t::OK);
   AgentInterface &agent = mockAgent.get();
   auto failedFollower = FailedFollower(
-    agency("arango"),
+    agency(PREFIX),
     &agent,
     JOB_STATUS::TODO,
     jobId
   );
-  failedFollower.start();
+  REQUIRE(failedFollower.start());
   Verify(Method(mockAgent, transact));
   Verify(Method(mockAgent, write));
 }
@@ -567,7 +569,7 @@ SECTION("a successfully started job should finish immediately and set everything
   When(Method(mockAgent, waitFor)).AlwaysReturn(AgentInterface::raft_commit_t::OK);
   AgentInterface &agent = mockAgent.get();
   auto failedFollower = FailedFollower(
-    agency("arango"),
+    agency(PREFIX),
     &agent,
     JOB_STATUS::TODO,
     jobId
@@ -669,7 +671,7 @@ SECTION("the job should handle distributeShardsLike") {
   When(Method(mockAgent, waitFor)).AlwaysReturn(AgentInterface::raft_commit_t::OK);
   AgentInterface &agent = mockAgent.get();
   auto failedFollower = FailedFollower(
-    agency("arango"),
+    agency(PREFIX),
     &agent,
     JOB_STATUS::TODO,
     jobId
@@ -736,7 +738,7 @@ SECTION("the job should timeout after a while") {
   When(Method(mockAgent, waitFor)).AlwaysReturn(AgentInterface::raft_commit_t::OK);
   AgentInterface &agent = mockAgent.get();
   auto failedFollower = FailedFollower(
-    agency("arango"),
+    agency(PREFIX),
     &agent,
     JOB_STATUS::TODO,
     jobId
@@ -791,7 +793,7 @@ SECTION("the job should be abortable when it is in todo") {
   When(Method(mockAgent, waitFor)).AlwaysReturn(AgentInterface::raft_commit_t::OK);
   AgentInterface &agent = mockAgent.get();
   auto failedFollower = FailedFollower(
-    agency("arango"),
+    agency(PREFIX),
     &agent,
     JOB_STATUS::TODO,
     jobId

tests/Agency/FailedLeaderTest.cpp

@@ -27,10 +27,9 @@
 #include "catch.hpp"
 #include "fakeit.hpp"
 
-#include "Agency/AddFollower.h"
 #include "Agency/FailedLeader.h"
-#include "Agency/MoveShard.h"
 #include "Agency/AgentInterface.h"
+#include "Agency/MoveShard.h"
 #include "Agency/Node.h"
 #include "lib/Basics/StringUtils.h"
 #include "lib/Random/RandomGenerator.h"
@@ -338,7 +337,9 @@ SECTION("if the leader is healthy again we fail the job") {
     JOB_STATUS::TODO,
     jobId
   );
-  failedLeader.start();
+  REQUIRE_FALSE(failedLeader.start());
+  Verify(Method(mockAgent, transact));
+  Verify(Method(mockAgent, write)).Exactly(Once);
 }
 
 SECTION("the job must not be started if there is no server that is in sync for every shard") {
@@ -382,7 +383,7 @@ SECTION("the job must not be started if there is no server that is in sync for e
     JOB_STATUS::TODO,
     jobId
   );
-  failedLeader.start();
+  REQUIRE_FALSE(failedLeader.start());
 }
 
 SECTION("the job must not be started if there if one of the linked shards (distributeShardsLike) is not in sync") {

tests/Agency/FailedServerTest.cpp

@@ -65,23 +65,6 @@ const char *agency =
 #include "FailedServerTest.json"
 ;
 
-VPackBuilder createJob() {
-  VPackBuilder builder;
-  {
-    VPackObjectBuilder a(&builder);
-    builder.add("creator", VPackValue("1"));
-    builder.add("type", VPackValue("failedServer"));
-    builder.add("database", VPackValue("database"));
-    builder.add("collection", VPackValue("collection"));
-    builder.add("shard", VPackValue("shard"));
-    builder.add("fromServer", VPackValue("follower1"));
-    builder.add("jobId", VPackValue("1"));
-    builder.add("timeCreated",
-                VPackValue(timepointToString(std::chrono::system_clock::now())));
-  }
-  return builder;
-}
-
 Node createNodeFromBuilder(VPackBuilder const& builder) {
 
   VPackBuilder opBuilder;
@@ -361,8 +344,7 @@ TEST_CASE("FailedServer", "[agency][supervision]") {
     
   } // SECTION
 
-
+  SECTION("The state is still 'FAILED' and 'Target/FailedServers' is PART 2") {
-    SECTION("The state is still 'FAILED' and 'Target/FailedServers' is PART 2") {
 
     TestStructureType createTestStructure = [&](
       Slice const& s, std::string const& path) {