From e12de2475a2447a2ecd2e4ac209aafb81cfef8d1 Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Thu, 03 Oct 2013 13:52:05 +0000
Subject: [PATCH] HistoricalTest failure: Code added a task but did not wait for it to complete before proceeding. Adding a wait should solve the problem.

---
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/ReplicationTestCase.java          |   45 ++---
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/ReplicationServerTest.java |   73 +++-----
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/tools/UpgradeTestCase.java                    |   68 ++------
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/TestCaseUtils.java                            |  113 +++-----------
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/HistoricalTest.java        |  115 +++++---------
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/AciBodyTest.java     |    6 
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/GenerationIdTest.java             |   45 +---
 7 files changed, 151 insertions(+), 314 deletions(-)

diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/TestCaseUtils.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/TestCaseUtils.java
index 0dd233a..0a20a5d 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/TestCaseUtils.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/TestCaseUtils.java
@@ -58,7 +58,6 @@
 import org.opends.server.protocols.asn1.ASN1;
 import org.opends.server.protocols.asn1.ASN1Reader;
 import org.opends.server.protocols.asn1.ASN1Writer;
-import org.opends.server.protocols.internal.InternalClientConnection;
 import org.opends.server.protocols.ldap.BindRequestProtocolOp;
 import org.opends.server.protocols.ldap.BindResponseProtocolOp;
 import org.opends.server.protocols.ldap.LDAPMessage;
@@ -71,6 +70,7 @@
 import org.opends.server.util.EmbeddedUtils;
 import org.opends.server.util.LDIFReader;
 
+import static org.opends.server.protocols.internal.InternalClientConnection.*;
 import static org.opends.server.util.ServerConstants.*;
 import static org.opends.server.util.StaticUtils.*;
 import static org.testng.Assert.*;
@@ -222,27 +222,19 @@
    * @see #shutdownFakeServer() Matching method that must be called in the test
    *      tear down.
    */
-  public static void startFakeServer()
+  public static void startFakeServer() throws Exception
   {
     schemaBeforeStartingFakeServer = DirectoryServer.getSchema();
     DirectoryServer.setSchema(initializeInMemory(new Schema()));
   }
 
-  private static Schema initializeInMemory(final Schema schema)
+  private static Schema initializeInMemory(final Schema schema) throws Exception
   {
-    try
+    for (AttributeType attributeType : AttributeTypeConstants.ALL)
     {
-      for (AttributeType attributeType : AttributeTypeConstants.ALL)
-      {
-        schema.registerAttributeType(attributeType, true);
-      }
-      return schema;
+      schema.registerAttributeType(attributeType, true);
     }
-    catch (DirectoryException e)
-    {
-      // rethrow this to fail the current test
-      throw new RuntimeException(e);
-    }
+    return schema;
   }
 
   /**
@@ -251,18 +243,9 @@
    * subsequent attempts to start it will be ignored because it will already be
    * available.
    *
-   * @throws  IOException  If a problem occurs while interacting with the
-   *                       filesystem to prepare the test package root.
-   *
-   * @throws  InitializationException  If a problem occurs while starting the
-   *                                   server.
-   *
-   * @throws  ConfigException  If there is a problem with the server
-   *                           configuration.
+   * @throws  Exception  If an unexpected problem occurs.
    */
-  public static void startServer()
-         throws IOException, InitializationException, ConfigException,
-                DirectoryException
+  public static void startServer() throws Exception
   {
     try {
       if (SERVER_STARTED)
@@ -513,19 +496,7 @@
       SERVER_STARTED = true;
 
       initializeTestBackend(true);
-    } catch (IOException e) {
-      e.printStackTrace(originalSystemErr);
-      throw e;
-    } catch (NumberFormatException e) {
-      e.printStackTrace(originalSystemErr);
-      throw e;
-    } catch (InitializationException e) {
-      e.printStackTrace(originalSystemErr);
-      throw e;
-    } catch (ConfigException e) {
-      e.printStackTrace(originalSystemErr);
-      throw e;
-    } catch (DirectoryException e) {
+    } catch (Exception e) {
       e.printStackTrace(originalSystemErr);
       throw e;
     }
@@ -823,12 +794,9 @@
    *
    * @throws  Exception  If an unexpected problem occurs.
    */
-  public static void initializeTestBackend(boolean createBaseEntry)
-         throws IOException, InitializationException, ConfigException,
-                DirectoryException
+  public static void initializeTestBackend(boolean createBaseEntry) throws Exception
   {
-    initializeMemoryBackend(
-        TEST_BACKEND_ID, TEST_ROOT_DN_STRING, createBaseEntry);
+    initializeMemoryBackend(TEST_BACKEND_ID, TEST_ROOT_DN_STRING, createBaseEntry);
   }
 
   /**
@@ -847,8 +815,7 @@
       String backendID,
       String namingContext,
       boolean createBaseEntry
-      ) throws IOException, InitializationException, ConfigException,
-                DirectoryException
+      ) throws Exception
   {
     startServer();
 
@@ -1293,16 +1260,9 @@
    *
    * @throws  Exception  If an unexpected problem occurs.
    */
-  public static void addEntry(Entry entry)
-         throws Exception
+  public static void addEntry(Entry entry) throws Exception
   {
-    InternalClientConnection conn =
-         InternalClientConnection.getRootConnection();
-
-    AddOperation addOperation = conn.processAdd(entry.getDN(),
-                                     entry.getObjectClasses(),
-                                     entry.getUserAttributes(),
-                                     entry.getOperationalAttributes());
+    AddOperation addOperation = getRootConnection().processAdd(entry);
     assertEquals(addOperation.getResultCode(), ResultCode.SUCCESS);
   }
 
@@ -1316,8 +1276,7 @@
    *
    * @throws  Exception  If an unexpected problem occurs.
    */
-  public static void deleteEntry(Entry entry)
-         throws Exception
+  public static void deleteEntry(Entry entry) throws Exception
   {
     deleteEntry(entry.getDN());
   }
@@ -1330,13 +1289,9 @@
    *
    * @throws  Exception  If an unexpected problem occurs.
    */
-  public static void deleteEntry(DN dn)
-         throws Exception
+  public static void deleteEntry(DN dn) throws Exception
   {
-    InternalClientConnection conn =
-         InternalClientConnection.getRootConnection();
-
-    DeleteOperation deleteOperation = conn.processDelete(dn);
+    DeleteOperation deleteOperation = getRootConnection().processDelete(dn);
     assertEquals(deleteOperation.getResultCode(), ResultCode.SUCCESS);
   }
 
@@ -1399,20 +1354,12 @@
    *
    * @throws  Exception  If an unexpected problem occurs.
    */
-  public static void addEntry(String... lines)
-         throws Exception
+  public static void addEntry(String... lines) throws Exception
   {
     Entry entry = makeEntry(lines);
-
-    InternalClientConnection conn =
-         InternalClientConnection.getRootConnection();
-
-    AddOperation addOperation = conn.processAdd(entry.getDN(),
-                                     entry.getObjectClasses(),
-                                     entry.getUserAttributes(),
-                                     entry.getOperationalAttributes());
-    assertEquals(addOperation.getResultCode(), ResultCode.SUCCESS, addOperation
-        .getErrorMessage().toString());
+    AddOperation addOperation = getRootConnection().processAdd(entry);
+    assertEquals(addOperation.getResultCode(), ResultCode.SUCCESS,
+        addOperation.getErrorMessage().toString());
   }
 
 
@@ -1427,18 +1374,10 @@
    *
    * @throws  Exception  If an unexpected problem occurs.
    */
-  public static ResultCode addEntryOperation(String... lines)
-         throws Exception
+  public static ResultCode addEntryOperation(String... lines) throws Exception
   {
     Entry entry = makeEntry(lines);
-
-    InternalClientConnection conn =
-         InternalClientConnection.getRootConnection();
-
-    AddOperation addOperation = conn.processAdd(entry.getDN(),
-                                     entry.getObjectClasses(),
-                                     entry.getUserAttributes(),
-                                     entry.getOperationalAttributes());
+    AddOperation addOperation = getRootConnection().processAdd(entry);
     return addOperation.getResultCode();
   }
 
@@ -1452,8 +1391,7 @@
    *
    * @throws  Exception  If an unexpected problem occurs.
    */
-  public static void addEntries(List<Entry> entries)
-         throws Exception
+  public static void addEntries(List<Entry> entries) throws Exception
   {
     for (Entry entry : entries)
     {
@@ -1473,8 +1411,7 @@
    *
    * @throws  Exception  If an unexpected problem occurs.
    */
-  public static void addEntries(String... lines)
-         throws Exception
+  public static void addEntries(String... lines) throws Exception
   {
     for (Entry entry : makeEntries(lines))
     {
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/AciBodyTest.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/AciBodyTest.java
index 71c9e7a..ae9b23f 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/AciBodyTest.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/AciBodyTest.java
@@ -26,8 +26,6 @@
  */
 package org.opends.server.authorization.dseecompat;
 
-import static org.assertj.core.api.Assertions.*;
-
 import org.opends.server.DirectoryServerTestCase;
 import org.opends.server.TestCaseUtils;
 import org.testng.annotations.AfterClass;
@@ -35,12 +33,14 @@
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
+import static org.assertj.core.api.Assertions.*;
+
 @SuppressWarnings("javadoc")
 public class AciBodyTest extends DirectoryServerTestCase
 {
 
   @BeforeClass
-  public void setUp()
+  public void setUp() throws Exception
   {
     TestCaseUtils.startFakeServer();
   }
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/GenerationIdTest.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/GenerationIdTest.java
index 6b36dc6..3309a9c 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/GenerationIdTest.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/GenerationIdTest.java
@@ -37,7 +37,6 @@
 import org.opends.messages.Severity;
 import org.opends.server.TestCaseUtils;
 import org.opends.server.backends.MemoryBackend;
-import org.opends.server.backends.task.TaskState;
 import org.opends.server.core.DirectoryServer;
 import org.opends.server.loggers.debug.DebugTracer;
 import org.opends.server.protocols.internal.InternalSearchOperation;
@@ -551,11 +550,8 @@
    */
   private void checkChangelogSize(int expectedCount) throws Exception
   {
-    SearchFilter filter = SearchFilter.createFilterFromString("(objectclass=*)");
-    InternalSearchOperation searchOperation =
-      connection.processSearch(DN.decode("dc=replicationchanges"),
-          SearchScope.SUBORDINATE_SUBTREE,
-          filter);
+    InternalSearchOperation searchOperation = connection.processSearch(
+        "dc=replicationchanges", SearchScope.SUBORDINATE_SUBTREE, "(objectclass=*)");
     if (debugEnabled())
     {
       if (searchOperation.getSearchEntries().size() != expectedCount)
@@ -652,17 +648,8 @@
       debugInfo(testCase + " ** TEST ** DS2 (bad genID) changes must be ignored.");
 
       broker2.publish(createAddMsg());
-      try
-      {
-        broker3.receive();
-        fail("No update message is supposed to be received here.");
-      }
-      catch (SocketTimeoutException expected)
-      {
-        // This is the expected result
-        // Note that timeout should be lower than RS monitoring publisher period
-        // so that timeout occurs
-      }
+      assertNoMessageReceived(broker3, "broker3",
+          "Note that timeout should be lower than RS monitoring publisher period so that timeout occurs");
 
       //===========================================================
       debugInfo(testCase + " ** TEST ** The part of the topology with the right gen ID should work well");
@@ -742,8 +729,7 @@
           "objectclass: ds-task-reset-generation-id",
           "ds-task-class-name: org.opends.server.tasks.SetGenerationIdTask",
           "ds-task-reset-generation-id-domain-base-dn: " + baseDnStr);
-      addTask(taskReset, ResultCode.SUCCESS, null);
-      waitTaskState(taskReset, TaskState.COMPLETED_SUCCESSFULLY, null);
+      executeTask(taskReset);
 
       // Broker 2 and 3 should receive 1 change status message to order them
       // to enter the bad gen id status
@@ -789,8 +775,8 @@
       Thread.sleep(500);
       checkChangelogSize(1);
 
-      assertNoMessageReceivedBadGenId(broker2, "broker2");
-      assertNoMessageReceivedBadGenId(broker3, "broker3");
+      assertNoMessageReceived(broker2, "broker2", "bad gen id");
+      assertNoMessageReceived(broker3, "broker3", "bad gen id");
 
       debugInfo("DS2 is publishing a change and RS1 must ignore this change, DS3 must not receive it.");
       AddMsg emsg = createAddMsg();
@@ -800,7 +786,7 @@
       Thread.sleep(500);
       checkChangelogSize(1);
 
-      assertNoMessageReceivedBadGenId(broker3, "broker3");
+      assertNoMessageReceived(broker3, "broker3", "bad gen id");
 
 
       //===============================================================
@@ -840,9 +826,7 @@
           "objectclass: ds-task-reset-generation-id",
           "ds-task-class-name: org.opends.server.tasks.SetGenerationIdTask",
           "ds-task-reset-generation-id-domain-base-dn: " + baseDnStr);
-
-      addTask(taskReset, ResultCode.SUCCESS, null);
-      waitTaskState(taskReset, TaskState.COMPLETED_SUCCESSFULLY, null);
+      executeTask(taskReset);
 
       debugInfo("Verify that RS1 has still the right genID");
       assertEquals(replServer1.getGenerationId(baseDN), rgenId);
@@ -893,13 +877,14 @@
     }
   }
 
-  private void assertNoMessageReceivedBadGenId(ReplicationBroker broker, String brokerName)
+  private void assertNoMessageReceived(ReplicationBroker broker,
+      String brokerName, String reason)
   {
     try
     {
       ReplicationMsg msg = broker.receive();
       fail("No update message is supposed to be received by " + brokerName
-          + " with bad gen id. " + msg);
+          + " reason='" + reason + "'. msg=" + msg);
     }
     catch (SocketTimeoutException expected)
     { /* expected */
@@ -999,8 +984,7 @@
         "objectclass: ds-task-reset-generation-id",
         "ds-task-class-name: org.opends.server.tasks.SetGenerationIdTask",
         "ds-task-reset-generation-id-domain-base-dn: " + baseDnStr);
-      addTask(taskReset, ResultCode.SUCCESS, null);
-      waitTaskState(taskReset, TaskState.COMPLETED_SUCCESSFULLY, null);
+      executeTask(taskReset);
       Thread.sleep(500);
 
       debugInfo("Verifying that all replservers genIds have been reset.");
@@ -1019,8 +1003,7 @@
         "ds-task-class-name: org.opends.server.tasks.SetGenerationIdTask",
         "ds-task-reset-generation-id-domain-base-dn: " + baseDnStr,
         "ds-task-reset-generation-id-new-value: -1");
-      addTask(taskReset, ResultCode.SUCCESS, null);
-      waitTaskState(taskReset, TaskState.COMPLETED_SUCCESSFULLY, null);
+      executeTask(taskReset);
 
       debugInfo("Verifying that all replservers genIds have been reset.");
       int waitRes = 0;
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/ReplicationTestCase.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/ReplicationTestCase.java
index dbb1073..0b37869 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/ReplicationTestCase.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/ReplicationTestCase.java
@@ -39,8 +39,7 @@
 import org.opends.server.backends.task.TaskState;
 import org.opends.server.config.ConfigException;
 import org.opends.server.core.AddOperation;
-import org.opends.server.core.AddOperationBasis;
-import org.opends.server.core.DeleteOperationBasis;
+import org.opends.server.core.DeleteOperation;
 import org.opends.server.core.DirectoryServer;
 import org.opends.server.loggers.debug.DebugTracer;
 import org.opends.server.protocols.internal.InternalClientConnection;
@@ -314,10 +313,7 @@
     if (dn.getParent().getRDN().toString().equalsIgnoreCase("cn=domains"))
       deleteEntry(DN.decode("cn=external changelog," + dn));
 
-    DeleteOperationBasis op = new DeleteOperationBasis(connection,
-        InternalClientConnection.nextOperationID(), InternalClientConnection.nextMessageID(),
-        null, dn);
-    op.run();
+    DeleteOperation op = connection.processDelete(dn);
     assertTrue(op.getResultCode() == SUCCESS || op.getResultCode() == NO_SUCH_OBJECT,
         "Delete entry " + dn + " failed: " + op.getResultCode().getResultCodeName());
   }
@@ -755,6 +751,12 @@
     return new ReplSessionSecurity(null, null, null, true);
   }
 
+  protected void executeTask(Entry taskEntry) throws Exception
+  {
+    addTask(taskEntry, ResultCode.SUCCESS, null);
+    waitTaskState(taskEntry, TaskState.COMPLETED_SUCCESSFULLY, null);
+  }
+
   /**
    * Add a task to the configuration of the current running DS.
    * @param taskEntry The task to add.
@@ -875,17 +877,7 @@
       for (String ldifEntry : ldifEntries)
       {
         Entry entry = TestCaseUtils.entryFromLdifString(ldifEntry);
-        AddOperationBasis addOp = new AddOperationBasis(
-            connection,
-            InternalClientConnection.nextOperationID(),
-            InternalClientConnection.nextMessageID(),
-            null,
-            entry.getDN(),
-            entry.getObjectClasses(),
-            entry.getUserAttributes(),
-            entry.getOperationalAttributes());
-        addOp.setInternalOperation(true);
-        addOp.run();
+        AddOperation addOp = connection.processAdd(entry);
         if (addOp.getResultCode() != ResultCode.SUCCESS)
         {
           TRACER.debugInfo("Failed to add entry " + entry.getDN() +
@@ -985,11 +977,11 @@
     assertTrue(session != null || broker != null, "One of Session or ReplicationBroker parameter must not be null");
     assertTrue(session == null || broker == null, "Only one of Session or ReplicationBroker parameter must not be null");
 
-    int timeOut = 5000; // 5 seconds max to wait for the desired message
-    long startTime = System.currentTimeMillis();
-    long curTime = startTime;
-    int nMsg = 0;
-    while ((curTime - startTime) <= timeOut)
+    final int timeOut = 5000; // 5 seconds max to wait for the desired message
+    final long startTime = System.currentTimeMillis();
+    final List<ReplicationMsg> msgs = new ArrayList<ReplicationMsg>();
+    boolean timedOut = false;
+    while (!timedOut)
     {
       ReplicationMsg replMsg = null;
       try
@@ -1015,13 +1007,12 @@
         return (T) replMsg;
       }
       TRACER.debugInfo("waitForSpecificMsg received : " + replMsg);
-      nMsg++;
-      curTime = System.currentTimeMillis();
+      msgs.add(replMsg);
+      timedOut = (System.currentTimeMillis() - startTime) > timeOut;
     }
     // Timeout
-    fail("Failed to receive an expected " + msgType
-        + " message after 5 seconds : also received " + nMsg
-        + " other messages during wait time.");
+    fail("Failed to receive an expected " + msgType + " message after 5 seconds."
+        + " Also received the following messages during wait time: " + msgs);
     return null;
   }
 }
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/HistoricalTest.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/HistoricalTest.java
index 4573dee..f444c43 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/HistoricalTest.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/plugin/HistoricalTest.java
@@ -28,15 +28,14 @@
 package org.opends.server.replication.plugin;
 
 import java.util.ArrayList;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.UUID;
 
+import org.assertj.core.api.Assertions;
 import org.opends.server.TestCaseUtils;
 import org.opends.server.core.DirectoryServer;
 import org.opends.server.protocols.internal.InternalClientConnection;
 import org.opends.server.protocols.internal.InternalSearchOperation;
-import org.opends.server.protocols.ldap.LDAPFilter;
 import org.opends.server.replication.ReplicationTestCase;
 import org.opends.server.replication.common.CSN;
 import org.opends.server.replication.protocol.AddMsg;
@@ -176,10 +175,8 @@
 
     // Check that encoding and decoding preserves the history information.
     EntryHistorical hist = EntryHistorical.newInstanceFromEntry(entry);
-    Attribute after = hist.encodeAndPurge();
-
     assertEquals(hist.getLastPurgedValuesCount(),0);
-    assertEquals(after, before);
+    assertEquals(hist.encodeAndPurge(), before);
 
     Thread.sleep(1000);
 
@@ -198,10 +195,9 @@
     entry = DirectoryServer.getEntry(dn);
     hist = EntryHistorical.newInstanceFromEntry(entry);
     hist.setPurgeDelay(testPurgeDelayInMillisec);
-    after = hist.encodeAndPurge();
 
     // The purge time is not done so the hist attribute should be not empty
-    assertTrue(!after.isEmpty());
+    assertFalse(hist.encodeAndPurge().isEmpty());
 
     // Now wait for the purge time to be done
     Thread.sleep(testPurgeDelayInMillisec + 200);
@@ -211,8 +207,7 @@
     entry = DirectoryServer.getEntry(dn);
     hist = EntryHistorical.newInstanceFromEntry(entry);
     hist.setPurgeDelay(testPurgeDelayInMillisec);
-    after = hist.encodeAndPurge();
-    assertTrue(after.isEmpty());
+    assertTrue(hist.encodeAndPurge().isEmpty());
     assertEquals(hist.getLastPurgedValuesCount(),11);
   }
 
@@ -261,10 +256,7 @@
        );
 
     // Read the entry back to get its UUID.
-    Entry entry = DirectoryServer.getEntry(dn1);
-    List<Attribute> attrs = entry.getAttribute(entryuuidType);
-    String entryuuid =
-         attrs.get(0).iterator().next().getValue().toString();
+    String entryuuid = getEntryValue(dn1, entryuuidType);
 
     // Add the second test entry.
     TestCaseUtils.addEntry(
@@ -279,10 +271,7 @@
        );
 
     // Read the entry back to get its UUID.
-    entry = DirectoryServer.getEntry(dn2);
-    attrs = entry.getAttribute(entryuuidType);
-    String entryuuid2 =
-         attrs.get(0).iterator().next().getValue().toString();
+    String entryuuid2 = getEntryValue(dn2, entryuuidType);
 
     long now = System.currentTimeMillis();
     // A change on a first server.
@@ -295,9 +284,7 @@
     // happen on one server.
 
     // Replay an add of a value A at time t1 on a first server.
-    Attribute attr = Attributes.create(attrType.getNormalizedPrimaryName(), "A");
-    Modification mod = new Modification(ModificationType.ADD, attr);
-    publishModify(broker, t1, dn1, entryuuid, mod);
+    publishModify(broker, t1, dn1, entryuuid, attrType, "A");
 
     // It would be nice to avoid these sleeps.
     // We need to preserve the replay order but the order could be changed
@@ -307,9 +294,7 @@
     Thread.sleep(2000);
 
     // Replay an add of a value B at time t2 on a second server.
-    attr = Attributes.create(attrType.getNormalizedPrimaryName(), "B");
-    mod = new Modification(ModificationType.ADD, attr);
-    publishModify(broker, t2, dn1, entryuuid, mod);
+    publishModify(broker, t2, dn1, entryuuid, attrType, "B");
 
     // Simulate the reverse ordering t2:add:B followed by t1:add:A that
     // would happen on the other server.
@@ -317,51 +302,44 @@
     t1 = new CSN(now+3,  0,  3);
     t2 = new CSN(now+4,  0,  4);
 
-    // Replay an add of a value B at time t2 on a second server.
-    attr = Attributes.create(attrType.getNormalizedPrimaryName(), "B");
-    mod = new Modification(ModificationType.ADD, attr);
-    publishModify(broker, t2, dn2, entryuuid2, mod);
+    publishModify(broker, t2, dn2, entryuuid2, attrType, "B");
 
     Thread.sleep(2000);
 
     // Replay an add of a value A at time t1 on a first server.
-    attr = Attributes.create(attrType.getNormalizedPrimaryName(), "A");
-    mod = new Modification(ModificationType.ADD, attr);
-    publishModify(broker, t1, dn2, entryuuid2, mod);
+    publishModify(broker, t1, dn2, entryuuid2, attrType, "A");
 
     Thread.sleep(2000);
 
-    // Read the first entry to see how the conflict was resolved.
-    entry = DirectoryServer.getEntry(dn1);
-    attrs = entry.getAttribute(attrType);
-    String attrValue1 =
-         attrs.get(0).iterator().next().getValue().toString();
-
-    // Read the second entry to see how the conflict was resolved.
-    entry = DirectoryServer.getEntry(dn2);
-    attrs = entry.getAttribute(attrType);
-    String attrValue2 =
-         attrs.get(0).iterator().next().getValue().toString();
-
+    // See how the conflicts were resolved.
     // The two values should be the first value added.
-    assertEquals(attrValue1, "A");
-    assertEquals(attrValue2, "A");
+    assertEquals(getEntryValue(dn1, attrType), "A");
+    assertEquals(getEntryValue(dn2, attrType), "A");
 
-    TestCaseUtils.deleteEntry(DN.decode("cn=test1," + TEST_ROOT_DN_STRING));
-    TestCaseUtils.deleteEntry(DN.decode("cn=test2," + TEST_ROOT_DN_STRING));
-}
+    TestCaseUtils.deleteEntry(dn1);
+    TestCaseUtils.deleteEntry(dn2);
+  }
 
-  private static
-  void publishModify(ReplicationBroker broker, CSN changeNum,
-                     DN dn, String entryuuid, Modification mod)
+  private String getEntryValue(final DN dn, final AttributeType attrType)
+      throws Exception
   {
+    Entry entry = DirectoryServer.getEntry(dn);
+    List<Attribute> attrs = entry.getAttribute(attrType);
+    return attrs.get(0).iterator().next().getValue().toString();
+  }
+
+  private static void publishModify(ReplicationBroker broker, CSN changeNum,
+      DN dn, String entryuuid, AttributeType attrType, String newValue)
+  {
+    Attribute attr = Attributes.create(attrType.getNormalizedPrimaryName(), newValue);
+    Modification mod = new Modification(ModificationType.ADD, attr);
     List<Modification> mods = new ArrayList<Modification>(1);
     mods.add(mod);
     broker.publish(new ModifyMsg(changeNum, dn, mods, entryuuid));
   }
 
   /**
-   * Test that historical information is correctly added when performaing ADD,
+   * Test that historical information is correctly added when performing ADD,
    * MOD and MODDN operations.
    */
   @Test()
@@ -471,19 +449,20 @@
         // - the dn should be dn1,
         // - the entry id and the parent id should match the ids from the entry
         FakeAddOperation addOp = (FakeAddOperation) op;
-        assertTrue(addOp.getCSN() != null);
+        assertNotNull(addOp.getCSN());
         AddMsg addmsg = addOp.generateMessage();
         assertEquals(dn1, addmsg.getDN());
         assertEquals(addmsg.getEntryUUID(), EntryHistorical.getEntryUUID(entry));
         String parentId = LDAPReplicationDomain.findEntryUUID(dn1.getParent());
         assertEquals(addmsg.getParentEntryUUID(), parentId);
+
         addmsg.createOperation(InternalClientConnection.getRootConnection());
       }
-      else if (count == 1)
+      else
       {
-          // The first operation should be an ADD operation.
-          fail("FakeAddOperation was not correctly generated"
-              + " from historical information");
+        // The first operation should be an ADD operation.
+        assertTrue(count != 1,
+            "FakeAddOperation was not correctly generated from historical information");
       }
     }
 
@@ -495,8 +474,8 @@
    * entry.
    * Steps :
    * - creates entry containing historical
-   * - wait for the pruge delay
-   * - lauch the purge task
+   * - wait for the purge delay
+   * - launch the purge task
    * - verify that all historical has been purged
    *
    * TODO: another test should be written that configures the task no NOT have
@@ -534,21 +513,14 @@
         "ds-task-purge-conflicts-historical-domain-dn: "+TEST_ROOT_DN_STRING,
     "ds-task-purge-conflicts-historical-maximum-duration: 120"); // 120 sec
 
-    addTask(taskInit, ResultCode.SUCCESS, null);
+    executeTask(taskInit);
 
     // every entry should be purged from its hist
       // Search for matching entries in config backend
-      InternalSearchOperation op = connection.processSearch(
-          ByteString.valueOf(TEST_ROOT_DN_STRING),
-          SearchScope.WHOLE_SUBTREE,
-          LDAPFilter.decode("(ds-sync-hist=*)"));
-      assertEquals(op.getResultCode(), ResultCode.SUCCESS,
-          op.getErrorMessage().toString());
-
-      // Check that no entries have been found
-      LinkedList<SearchResultEntry> entries = op.getSearchEntries();
-      assertTrue(entries != null);
-      assertEquals(entries.size(), 0);
+    InternalSearchOperation op = connection.processSearch(
+        TEST_ROOT_DN_STRING, SearchScope.WHOLE_SUBTREE, "(ds-sync-hist=*)");
+    assertEquals(op.getResultCode(), ResultCode.SUCCESS, op.getErrorMessage().toString());
+    Assertions.assertThat(op.getSearchEntries()).isEmpty();
   }
 
   /**
@@ -556,8 +528,7 @@
    * @param dnSuffix A suffix to be added to the dn
    * @param entryCnt The number of entries to create
    */
-  private void addEntriesWithHistorical(int dnSuffix, int entryCnt)
-  throws Exception
+  private void addEntriesWithHistorical(int dnSuffix, int entryCnt) throws Exception
   {
     for (int i=0; i<entryCnt;i++)
     {
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/ReplicationServerTest.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/ReplicationServerTest.java
index 1d92cca..2f1bc53 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/ReplicationServerTest.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/replication/server/ReplicationServerTest.java
@@ -37,7 +37,6 @@
 import org.assertj.core.api.Assertions;
 import org.opends.server.TestCaseUtils;
 import org.opends.server.api.SynchronizationProvider;
-import org.opends.server.backends.task.TaskState;
 import org.opends.server.core.DirectoryServer;
 import org.opends.server.core.ModifyDNOperationBasis;
 import org.opends.server.loggers.debug.DebugTracer;
@@ -279,9 +278,7 @@
 
   private void assertDeleteMsgBodyEquals(DeleteMsg sentMsg, ReplicationMsg receivedMsg)
   {
-    assertTrue(receivedMsg instanceof DeleteMsg,
-        "ReplicationServer basic : incorrect message type received: "
-            + receivedMsg.getClass() + ": content: " + receivedMsg);
+    Assertions.assertThat(receivedMsg).isInstanceOf(DeleteMsg.class);
     assertEquals(receivedMsg.toString(), sentMsg.toString(),
         "ReplicationServer basic : incorrect message body received. CSN is same as \""
             + getCSNFieldName(((DeleteMsg) receivedMsg).getCSN()) + "\" field.");
@@ -378,16 +375,10 @@
    */
   private void assertDeleteMsgCSNEquals(ReplicationMsg msg, CSN nextCSN, String msgNumber)
   {
-    if (msg instanceof DeleteMsg)
-    {
-      DeleteMsg del = (DeleteMsg) msg;
-      assertEquals(del.getCSN(), nextCSN, "The " + msgNumber
-          + " message received by a new client was the wrong one.");
-    }
-    else
-    {
-      fail("ReplicationServer basic transmission failed:" + msg);
-    }
+    Assertions.assertThat(msg).isInstanceOf(DeleteMsg.class);
+    DeleteMsg del = (DeleteMsg) msg;
+    assertEquals(del.getCSN(), nextCSN, "The " + msgNumber
+        + " message received by a new client was the wrong one.");
   }
 
   /**
@@ -940,7 +931,7 @@
 
       // Read the TopologyMsg that should come back.
       ReplicationMsg repMsg = session.receive();
-      assertTrue(repMsg instanceof TopologyMsg);
+      Assertions.assertThat(repMsg).isInstanceOf(TopologyMsg.class);
 
       // Now comes the real test : check that the Replication Server
       // answers correctly to a WindowProbeMsg Message.
@@ -1115,31 +1106,28 @@
      Entry backupTask = createBackupTask();
      Entry restoreTask = createRestoreTask();
 
-     addTask(backupTask, ResultCode.SUCCESS, null);
-     waitTaskState(backupTask, TaskState.COMPLETED_SUCCESSFULLY, null);
-
-     addTask(restoreTask, ResultCode.SUCCESS, null);
-     waitTaskState(restoreTask, TaskState.COMPLETED_SUCCESSFULLY, null);
+    executeTask(backupTask);
+    executeTask(restoreTask);
 
      debugInfo("Ending backupRestore");
    }
 
-   /*
+   /**
     * Test export of the Replication server backend
-    * - Creates 2 brokers connecting to the replication for 2 differents baseDN
+    * - Creates 2 brokers connecting to the replication for 2 different baseDN
     * - Make these brokers publish changes to the replication server
     * - Launch a full export
     * - Launch a partial export on one of the 2 domains
     */
-    private void exportBackend() throws Exception
-    {
+  private void exportBackend() throws Exception
+  {
       debugInfo("Starting exportBackend");
 
       ReplicationBroker server1 = null;
       ReplicationBroker server2 = null;
 
-      try
-      {
+    try
+    {
         server1 = openReplicationSession(TEST_ROOT_DN,
             1, 100, replicationServerPort, 1000, true);
         server2 = openReplicationSession(DN.decode("dc=domain2,dc=com"),
@@ -1153,30 +1141,25 @@
         publishAll(server2, createChanges("dc=domain2,dc=com",  2));
 
         debugInfo("Export all");
-        Entry exportTask = createExportAllTask();
-        addTask(exportTask, ResultCode.SUCCESS, null);
-        waitTaskState(exportTask, TaskState.COMPLETED_SUCCESSFULLY, null);
-        // Not doing anything with the export file, let's delete it
-        File f = new File(DirectoryServer.getInstanceRoot(),exportLDIFAllFile);
-        f.delete();
+      executeTask(createExportAllTask());
+      // Not doing anything with the export file, let's delete it
+      new File(DirectoryServer.getInstanceRoot(), exportLDIFAllFile).delete();
 
         debugInfo("Export domain");
-        exportTask = createExportDomainTask("dc=domain2,dc=com");
-        addTask(exportTask, ResultCode.SUCCESS, null);
-        waitTaskState(exportTask, TaskState.COMPLETED_SUCCESSFULLY, null);
+      executeTask(createExportDomainTask("dc=domain2,dc=com"));
+      if (exportLDIFDomainFile != null)
+      {
         // Not doing anything with the export file, let's delete it
-        if (exportLDIFDomainFile != null)
-        {
-          File aFile = new File(DirectoryServer.getInstanceRoot(),
-                  exportLDIFDomainFile);
-          aFile.delete();
-        }
-      } finally {
-      stop(server1, server2);
+        new File(DirectoryServer.getInstanceRoot(), exportLDIFDomainFile).delete();
       }
-
-      debugInfo("Ending export");
     }
+    finally
+    {
+      stop(server1, server2);
+    }
+
+    debugInfo("Ending export");
+  }
 
   private Entry createBackupTask() throws Exception
   {
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/tools/UpgradeTestCase.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/tools/UpgradeTestCase.java
index 5a7661d..922cdc5 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/tools/UpgradeTestCase.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/tools/UpgradeTestCase.java
@@ -28,32 +28,29 @@
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
-import java.io.IOException;
 import java.io.PrintStream;
 import java.util.LinkedList;
 import java.util.List;
 
+import org.assertj.core.api.Assertions;
 import org.opends.messages.Message;
 import org.opends.server.TestCaseUtils;
-import org.opends.server.config.ConfigException;
 import org.opends.server.core.DirectoryServer;
 import org.opends.server.tools.upgrade.UpgradeCli;
-import org.opends.server.types.DirectoryException;
-import org.opends.server.types.InitializationException;
 import org.opends.server.util.StaticUtils;
 import org.testng.annotations.Test;
 
 import static org.opends.messages.ToolMessages.*;
-import static org.opends.server.tools.ToolConstants.OPTION_LONG_FORCE_UPGRADE;
+import static org.opends.server.tools.ToolConstants.*;
 import static org.testng.Assert.*;
 
 /**
  * A set of test cases for the Upgrade tool.
  */
+@SuppressWarnings("javadoc")
 public class UpgradeTestCase extends ToolsTestCase
 {
-  private final static String configFilePath = DirectoryServer
-      .getInstanceRoot()
+  private final static String configFilePath = DirectoryServer.getInstanceRoot()
       + File.separator + "config" + File.separator + "config.ldif";
 
   /**
@@ -83,7 +80,6 @@
     }
     final String[] mainArgs = new String[argsList.size()];
     argsList.toArray(mainArgs);
-
     return mainArgs;
   }
 
@@ -94,13 +90,12 @@
    *          The upgrade's output.
    * @param expectedMessage
    *          The expected message.
-   * @return {@code true} if the output contains the expected message.
    */
-  private boolean isOutputContainsExpectedMessage(final String output,
-      final Message expectedMessage)
+  private void assertContainsMessage(String output, Message expectedMessage)
   {
-    return (output.replaceAll("\n", " ").replaceAll("%s", " ").indexOf(
-        expectedMessage.toString().replaceAll("\n", " ").replaceAll("%s", " ")) != -1);
+    String out = output.replaceAll("\n", " ").replaceAll("%s", " ");
+    String expected = expectedMessage.toString().replaceAll("\n", " ").replaceAll("%s", " ");
+    Assertions.assertThat(out).contains(expected);
   }
 
   /**
@@ -116,9 +111,7 @@
     {
       // The 'main' should exit with success code.
       assertEquals(UpgradeCli.main(setArgs("--help"), true, ps, ps), 0);
-
-      assertTrue(isOutputContainsExpectedMessage(baos.toString(),
-          INFO_UPGRADE_DESCRIPTION_CLI.get()));
+      assertContainsMessage(baos.toString(), INFO_UPGRADE_DESCRIPTION_CLI.get());
     }
     finally
     {
@@ -139,9 +132,7 @@
     {
       // The 'main' should exit with success code.
       assertEquals(UpgradeCli.main(setArgs("-H"), true, ps, ps), 0);
-
-      assertTrue(isOutputContainsExpectedMessage(baos.toString(),
-          INFO_UPGRADE_DESCRIPTION_CLI.get()));
+      assertContainsMessage(baos.toString(), INFO_UPGRADE_DESCRIPTION_CLI.get());
     }
     finally
     {
@@ -162,9 +153,7 @@
     {
       // The 'main' should exit with success code.
       assertEquals(UpgradeCli.main(setArgs("-?"), true, ps, ps), 0);
-
-      assertTrue(isOutputContainsExpectedMessage(baos.toString(),
-          INFO_UPGRADE_DESCRIPTION_CLI.get()));
+      assertContainsMessage(baos.toString(), INFO_UPGRADE_DESCRIPTION_CLI.get());
     }
     finally
     {
@@ -185,9 +174,7 @@
     {
       // The 'main' should exit with an error code.
       assertEquals(UpgradeCli.main(setArgs("-- wrong"), true, ps, ps), 1);
-
-      assertTrue(isOutputContainsExpectedMessage(baos.toString(),
-          ERR_ERROR_PARSING_ARGS.get("")));
+      assertContainsMessage(baos.toString(), ERR_ERROR_PARSING_ARGS.get(""));
     }
     finally
     {
@@ -208,9 +195,7 @@
     {
       // The 'main' should exit with an error code.
       assertEquals(UpgradeCli.main(setArgs("--wrong"), true, ps, ps), 1);
-
-      assertTrue(isOutputContainsExpectedMessage(baos.toString(),
-          ERR_ERROR_PARSING_ARGS.get("")));
+      assertContainsMessage(baos.toString(), ERR_ERROR_PARSING_ARGS.get(""));
     }
     finally
     {
@@ -233,11 +218,8 @@
       assertEquals(UpgradeCli.main(setArgs("--force"), true, ps, ps), 1);
 
       // Because interactive mode is not compatible with force upgrade mode.
-      final Message message =
-          ERR_UPGRADE_INCOMPATIBLE_ARGS.get(OPTION_LONG_FORCE_UPGRADE,
-              "interactive mode");
-
-      assertTrue(isOutputContainsExpectedMessage(baos.toString(), message));
+      assertContainsMessage(baos.toString(), ERR_UPGRADE_INCOMPATIBLE_ARGS.get(
+          OPTION_LONG_FORCE_UPGRADE, "interactive mode"));
     }
     finally
     {
@@ -247,16 +229,9 @@
 
   /**
    * Upgrade tool allows use of force and no-prompt sub-commands.
-   *
-   * @throws IOException
-   * @throws DirectoryException
-   * @throws ConfigException
-   * @throws InitializationException
    */
   @Test()
-  public void testUpgradeToolAllowsNonInteractiveAndForce()
-      throws InitializationException, ConfigException, DirectoryException,
-      IOException
+  public void testUpgradeToolAllowsNonInteractiveAndForce() throws Exception
   {
     TestCaseUtils.startServer();
 
@@ -265,20 +240,17 @@
     try
     {
       // The 'main' should exit with success code.
-      assertEquals(UpgradeCli.main(setArgs("--force", "--no-prompt"), true, ps,
-          ps), 0);
+      int rc = UpgradeCli.main(setArgs("--force", "--no-prompt"), true, ps, ps);
+      assertEquals(rc, 0);
 
       // The sub-commands have been checked ok but upgrade must exist on
       // version's verification.
-      assertTrue(isOutputContainsExpectedMessage(baos.toString(),
-          ERR_UPGRADE_VERSION_UP_TO_DATE.get("")));
-
+      assertContainsMessage(baos.toString(), ERR_UPGRADE_VERSION_UP_TO_DATE.get(""));
     }
     finally
     {
       StaticUtils.close(ps, baos);
-      TestCaseUtils
-          .shutdownServer("testUpgradeToolAllowsNonInteractiveAndForce");
+      TestCaseUtils.shutdownServer("testUpgradeToolAllowsNonInteractiveAndForce");
     }
   }
 }

--
Gitblit v1.10.0