Merge pull request igniterealtime#584 from guusdk/sint_human-readable…

…-assertion-messages

[sinttest] Assertions to have human readable messages

smack-integration-test/src/main/java/org/jivesoftware/smack/LoginIntegrationTest.java

@@ -64,7 +64,8 @@ public void testInvalidLogin(UnconnectedConnectionSource unconnectedConnectionSo
                             () -> connection.login(nonExistentUserString, invalidPassword));
 
             SaslNonza.SASLFailure saslFailure = saslErrorException.getSASLFailure();
-            assertEquals(SASLError.not_authorized, saslFailure.getSASLError());
+            assertEquals(SASLError.not_authorized, saslFailure.getSASLError(),
+        "Expected the server to return the appropriate SASL failure condition (but it did not)");
         } finally {
             connection.disconnect();
         }

smack-integration-test/src/main/java/org/jivesoftware/smack/StreamManagementTest.java

@@ -32,6 +32,7 @@
 import org.igniterealtime.smack.inttest.SmackIntegrationTestEnvironment;
 import org.igniterealtime.smack.inttest.TestNotPossibleException;
 import org.igniterealtime.smack.inttest.annotations.SmackIntegrationTest;
+import org.jxmpp.jid.EntityFullJid;
 
 public class StreamManagementTest extends AbstractSmackSpecificLowLevelIntegrationTest<XMPPTCPConnection> {
 
@@ -57,18 +58,18 @@ public void testStreamManagement(XMPPTCPConnection conOne, XMPPTCPConnection con
 
         try {
             send(body1, conOne, conTwo);
-            assertMessageWithBodyReceived(body1, collector);
+            assertMessageWithBodyReceived(body1, collector, conTwo.getUser());
 
             conOne.instantShutdown();
 
             send(body2, conOne, conTwo);
 
             // Reconnect with xep198
             conOne.connect().login();
-            assertMessageWithBodyReceived(body2, collector);
+            assertMessageWithBodyReceived(body2, collector, conTwo.getUser());
 
             send(body3, conOne, conTwo);
-            assertMessageWithBodyReceived(body3, collector);
+            assertMessageWithBodyReceived(body3, collector, conTwo.getUser());
         }
         finally {
             collector.cancel();
@@ -84,9 +85,9 @@ private static void send(String messageString, XMPPConnection from, XMPPConnecti
         from.sendStanza(message);
     }
 
-    private static void assertMessageWithBodyReceived(String body, StanzaCollector collector) throws InterruptedException {
+    private static void assertMessageWithBodyReceived(String body, StanzaCollector collector, EntityFullJid recipient) throws InterruptedException {
         Message message = collector.nextResult();
-        assertNotNull(message);
-        assertEquals(body, message.getBody());
+        assertNotNull(message, "Expected '" + recipient + "' to receive a message stanza with body '" + body + "', but it didn't receive the message stanza at all.");
+        assertEquals(body, message.getBody(), "Expected '" + recipient + "'to receive a message stanza with a specific body, but it received a message stanza with a different body.");
     }
 }

smack-integration-test/src/main/java/org/jivesoftware/smack/WaitForClosingStreamElementTest.java

@@ -39,6 +39,6 @@ public void waitForClosingStreamElementTest(AbstractXMPPConnection connection)
         Field closingStreamReceivedField = AbstractXMPPConnection.class.getDeclaredField("closingStreamReceived");
         closingStreamReceivedField.setAccessible(true);
         boolean closingStreamReceived = (boolean) closingStreamReceivedField.get(connection);
-        assertTrue(closingStreamReceived);
+        assertTrue(closingStreamReceived, "Expected to, but did not, receive a closing stream element on connection " + connection);
     }
 }

smack-integration-test/src/main/java/org/jivesoftware/smack/roster/RosterIntegrationTest.java

@@ -16,8 +16,6 @@
  */
 package org.jivesoftware.smack.roster;
 
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
 import java.util.Collection;
 import java.util.concurrent.TimeoutException;
 
@@ -78,16 +76,16 @@ private void checkIfAddedAndSubscribed(Collection<Jid> addresses) {
                     BareJid bareJid = conTwo.getUser().asBareJid();
                     RosterEntry rosterEntry = rosterOne.getEntry(bareJid);
                     if (rosterEntry == null) {
-                        addedAndSubscribed.signalFailure("No roster entry for " + bareJid);
+                        addedAndSubscribed.signalFailure("Added/Updated entry was not for " + bareJid);
                         return;
                     }
                     String name = rosterEntry.getName();
                     if (StringUtils.isNullOrEmpty(name)) {
-                        addedAndSubscribed.signalFailure("Roster entry without name");
+                        addedAndSubscribed.signalFailure("Added/Updated entry without name");
                         return;
                     }
                     if (!rosterEntry.getName().equals(conTwosRosterName)) {
-                        addedAndSubscribed.signalFailure("Roster name does not match");
+                        addedAndSubscribed.signalFailure("Added/Updated entry name does not match. Expected: " + conTwosRosterName + " but was: " + rosterEntry.getName());
                         return;
                     }
                     if (!rosterEntry.getType().equals(ItemType.to)) {
@@ -100,8 +98,9 @@ private void checkIfAddedAndSubscribed(Collection<Jid> addresses) {
 
         try {
             rosterOne.createItemAndRequestSubscription(conTwo.getUser().asBareJid(), conTwosRosterName, null);
-
-            assertTrue(addedAndSubscribed.waitForResult(2 * connection.getReplyTimeout()));
+            assertResult(addedAndSubscribed, 2 * connection.getReplyTimeout(),
+                "A roster entry for " + conTwo.getUser().asBareJid() + " using the name '" + conTwosRosterName +
+                "' of type 'to' was expected to be added to the roster of " + conOne.getUser() + " (but it was not).");
         }
         finally {
             rosterTwo.removeSubscribeListener(subscribeListener);

smack-integration-test/src/main/java/org/jivesoftware/smackx/caps/EntityCapsTest.java

@@ -18,6 +18,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -96,7 +97,9 @@ public void tearDown() throws NotConnectedException, InterruptedException {
     public void testLocalEntityCaps() throws InterruptedException, NoResponseException, XMPPErrorException, NotConnectedException {
         final String dummyFeature = getNewDummyFeature();
         DiscoverInfo info = EntityCapsManager.getDiscoveryInfoByNodeVer(ecmTwo.getLocalNodeVer());
-        assertFalse(info.containsFeature(dummyFeature));
+        assertFalse(info.containsFeature(dummyFeature),
+            "Expected the service discovery info for node '" + ecmTwo.getLocalNodeVer() +
+            "' to contain the feature '" + dummyFeature + "' (but it did not)."); // TODO Shouldn't this assertion be in a unit test instead of an integration test?
 
         dropWholeEntityCapsCache();
 
@@ -120,8 +123,12 @@ public void run() {
         // The other connection has to receive this stanza and record the
         // information in order for this test to succeed.
         info = EntityCapsManager.getDiscoveryInfoByNodeVer(ecmTwo.getLocalNodeVer());
-        assertNotNull(info);
-        assertTrue(info.containsFeature(dummyFeature));
+        assertNotNull(info,
+    "Expected '" + conOne.getUser() + "' to have received an 'available' presence from '" + conTwo.getUser() +
+            "' with a new CAPS 'ver' attribute (but it did not).");
+        assertTrue(info.containsFeature(dummyFeature),
+    "Expected the service discovery info for node '" + ecmTwo.getLocalNodeVer() +
+            "' to contain the feature '" + dummyFeature + "' (but it did not)."); // TODO As above: shouldn't this assertion be in a unit test instead of an integration test?
     }
 
     /**
@@ -148,16 +155,17 @@ public void processStanza(Stanza stanza) {
         // discover that
         DiscoverInfo info = sdmOne.discoverInfo(conTwo.getUser());
         // that discovery should cause a disco#info
-        assertTrue(discoInfoSend.get());
+        assertTrue(discoInfoSend.get(), "Expected '" + conOne.getUser() + "' to have made a disco/info request to '" + conTwo.getUser() + "', but it did not.");
         assertTrue(info.containsFeature(dummyFeature),
                         "The info response '" + info + "' does not contain the expected feature '" + dummyFeature + '\'');
         discoInfoSend.set(false);
 
         // discover that
         info = sdmOne.discoverInfo(conTwo.getUser());
         // that discovery shouldn't cause a disco#info
-        assertFalse(discoInfoSend.get());
-        assertTrue(info.containsFeature(dummyFeature));
+        assertFalse(discoInfoSend.get(), "Expected '" + conOne.getUser() + "' to not have made a disco/info request to '" + conTwo.getUser() + "' (as CAPS should have been cached), but it did not.");
+        assertTrue(info.containsFeature(dummyFeature),
+            "The info response '" + info + "' does not contain the expected feature '" + dummyFeature + '\'');
     }
 
     @SmackIntegrationTest
@@ -167,7 +175,8 @@ public void testCapsChanged() throws Exception {
         addFeatureAndWaitForPresence(conOne, conTwo, dummyFeature);
         String nodeVerAfter = EntityCapsManager.getNodeVersionByJid(conTwo.getUser());
 
-        assertFalse(nodeVerBefore.equals(nodeVerAfter));
+        assertNotEquals(nodeVerBefore, nodeVerAfter,
+            "Expected the reported node 'ver' value to differ after a feature was added (but it did not).");
     }
 
     @SmackIntegrationTest
@@ -193,12 +202,12 @@ public boolean evaluate() throws NoResponseException, XMPPErrorException, NotCon
         DiscoverInfo info = sdmOne.discoverInfo(conTwo.getUser());
 
         String u1ver = EntityCapsManager.getNodeVersionByJid(conTwo.getUser());
-        assertNotNull(u1ver);
+        assertNotNull(u1ver, "Expected " + conOne.getUser() + " to have received a CAPS 'ver' value for " + conTwo.getUser() + " (but did not).");
 
         DiscoverInfo entityInfo = EntityCapsManager.CAPS_CACHE.lookup(u1ver);
-        assertNotNull(entityInfo);
+        assertNotNull(entityInfo, "Expected the local static cache to have a value cached for 'ver' value '" + u1ver + "' (but it did not).");
 
-        assertEquals(info.toXML().toString(), entityInfo.toXML().toString());
+        assertEquals(info.toXML().toString(), entityInfo.toXML().toString(), "Expected the cached service/discovery info to be equal to the original (but it was not).");
     }
 
     private static void dropWholeEntityCapsCache() {

smack-integration-test/src/main/java/org/jivesoftware/smackx/chatstate/ChatStateIntegrationTest.java

@@ -75,7 +75,7 @@ public void testChatStateListeners() throws Exception {
         Chat chat = ChatManager.getInstanceFor(conOne)
                 .chatWith(conTwo.getUser().asEntityBareJid());
         chat.send("Hi!");
-        activeSyncPoint.waitForResult(timeout);
+        assertResult(activeSyncPoint, "Expected " + conTwo.getUser() + " to receive an 'active' chat state from " + conOne + " (but they did not).");
     }
 
     @AfterClass

smack-integration-test/src/main/java/org/jivesoftware/smackx/commands/AdHocCommandIntegrationTest.java

@@ -73,7 +73,9 @@ public AdHocCommandData executeSingleStage(AdHocCommandDataBuilder response) {
             AdHocCommandData response = result.getResponse();
             DataForm form = response.getForm();
             FormField field = form.getField("my-field");
-            assertNotNull(field);
+            assertNotNull(field, "Expected a field named 'my-field' to exist in the form that " +
+                conTwo.getUser() + " obtained from " + conOne.getUser() + "'s command node '" + commandNode +
+                "' (but it did not).");
         } finally {
             manOne.unregisterCommand(commandNode);
         }
@@ -259,7 +261,10 @@ public void multiStageAdHocCommandTest() throws NoResponseException, XMPPErrorEx
             AdHocCommandResult.StatusCompleted completed = command.complete(submitForm).asCompletedOrThrow();
 
             String operationResult = completed.getResponse().getForm().getField("result").getFirstValue();
-            assertEquals("65", operationResult);
+            assertEquals("65", operationResult,
+                "Unexpected value in the field 'result' from the command result that " + conTwo.getUser() +
+                " received from " + conOne.getUser() + " after completing a multi-staged ad-hoc command on node '" +
+                commandNode + "'.");
         } finally {
             manTwo.unregisterCommand(commandNode);
         }
@@ -317,7 +322,10 @@ public void multiStageWithPrevAdHocCommandTest() throws NoResponseException, XMP
             AdHocCommandResult.StatusCompleted completed = command.complete(submitForm).asCompletedOrThrow();
 
             String operationResult = completed.getResponse().getForm().getField("result").getFirstValue();
-            assertEquals("100", operationResult);
+            assertEquals("100", operationResult,
+                "Unexpected value in the field 'result' from the command result that " + conTwo.getUser() +
+                " received from " + conOne.getUser() + " after completing a multi-staged ad-hoc command on node '" +
+                commandNode + "'.");
         } finally {
             manTwo.unregisterCommand(commandNode);
         }
@@ -346,7 +354,9 @@ public void multiStageInvalidArgAdHocCommandTest() throws NoResponseException, X
             SubmitForm submitForm = form.getSubmitForm();
 
             XMPPErrorException exception = assertThrows(XMPPErrorException.class, () -> command.next(submitForm));
-            assertEquals(exception.getStanzaError().getCondition(), StanzaError.Condition.bad_request);
+            assertEquals(exception.getStanzaError().getCondition(), StanzaError.Condition.bad_request,
+        "Unexpected error condition received after " + conTwo.getUser() + " supplied an invalid argument " +
+                "to the command node '" + commandNode + "' of " + conOne.getUser());
         } finally {
             manTwo.unregisterCommand(commandNode);
         }

smack-integration-test/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferIntegrationTest.java

@@ -32,7 +32,7 @@
 import org.igniterealtime.smack.inttest.SmackIntegrationTestEnvironment;
 import org.igniterealtime.smack.inttest.annotations.SmackIntegrationTest;
 import org.igniterealtime.smack.inttest.annotations.SpecificationReference;
-import org.igniterealtime.smack.inttest.util.ResultSyncPoint;
+import org.igniterealtime.smack.inttest.util.SimpleResultSyncPoint;
 
 @SpecificationReference(document = "XEP-0096")
 public class FileTransferIntegrationTest extends AbstractSmackIntegrationTest {
@@ -67,7 +67,7 @@ public void ibbFileTransferTest() throws Exception {
     }
 
     private void genericfileTransferTest() throws Exception {
-        final ResultSyncPoint<String, Exception> resultSyncPoint = new ResultSyncPoint<>();
+        final SimpleResultSyncPoint resultSyncPoint = new SimpleResultSyncPoint();
         final FileTransferListener receiveListener = new FileTransferListener() {
             @Override
             public void fileTransferRequest(FileTransferRequest request) {
@@ -84,7 +84,7 @@ public void fileTransferRequest(FileTransferRequest request) {
                     os.flush();
                     dataReceived = os.toByteArray();
                     if (Arrays.equals(dataToSend, dataReceived)) {
-                        resultSyncPoint.signal("Received data matches send data. \\o/");
+                        resultSyncPoint.signal();
                     }
                     else {
                         resultSyncPoint.signal(new Exception("Received data does not match"));
@@ -117,7 +117,9 @@ public void fileTransferRequest(FileTransferRequest request) {
             }
         }
 
-        resultSyncPoint.waitForResult(MAX_FT_DURATION * 1000);
+        assertResult(resultSyncPoint, MAX_FT_DURATION * 1000,
+    "Expected data to be transferred successfully from " + conOne.getUser() + " to " + conTwo.getUser() +
+            " (but it did not).");
 
         ftManagerTwo.removeFileTransferListener(receiveListener);
     }

smack-integration-test/src/main/java/org/jivesoftware/smackx/geolocation/GeolocationIntegrationTest.java

@@ -17,7 +17,6 @@
 package org.jivesoftware.smackx.geolocation;
 
 import java.net.URI;
-import java.util.concurrent.TimeoutException;
 
 import org.jivesoftware.smack.SmackException.NoResponseException;
 import org.jivesoftware.smack.SmackException.NotConnectedException;
@@ -37,7 +36,6 @@
 import org.igniterealtime.smack.inttest.annotations.SpecificationReference;
 import org.igniterealtime.smack.inttest.util.IntegrationTestRosterUtil;
 import org.igniterealtime.smack.inttest.util.SimpleResultSyncPoint;
-import org.junit.jupiter.api.Assertions;
 import org.jxmpp.util.XmppDateTime;
 
 @SpecificationReference(document = "XEP-0080")
@@ -108,14 +106,9 @@ public void testNotification() throws Exception {
             glm1.publishGeoLocation(data); // for the purpose of this test, this needs not be blocking/use publishAndWait();
 
             // Wait for the data to be received.
-            try {
-                Object result = geoLocationReceived.waitForResult(timeout);
-
-                // Explicitly assert the success case.
-                Assertions.assertNotNull(result, "Expected to receive a PEP notification, but did not.");
-            } catch (TimeoutException e) {
-                Assertions.fail("Expected to receive a PEP notification, but did not.");
-            }
+            assertResult(geoLocationReceived,
+        "Expected " + conTwo.getUser() + " to receive a PEP notification from " + conOne.getUser() +
+                " that contained '" + data.toXML() + "', but did not.");
         } finally {
             unregisterListener(glm2, geoLocationListener);
         }
@@ -173,14 +166,9 @@ public void testNotificationAfterFilterChange() throws Exception {
             registerListenerAndWait(glm2, ServiceDiscoveryManager.getInstanceFor(conTwo), geoLocationListener);
 
             // Wait for the data to be received.
-            try {
-                Object result = geoLocationReceived.waitForResult(timeout);
-
-                // Explicitly assert the success case.
-                Assertions.assertNotNull(result, "Expected to receive a PEP notification, but did not.");
-            } catch (TimeoutException e) {
-                Assertions.fail("Expected to receive a PEP notification, but did not.");
-            }
+            assertResult(geoLocationReceived,
+        "Expected " + conTwo.getUser() + " to receive a PEP notification from " + conOne.getUser() +
+                " that contained '" + data.toXML() + "', but did not.");
         } finally {
             unregisterListener(glm2, geoLocationListener);
         }

smack-integration-test/src/main/java/org/jivesoftware/smackx/httpfileupload/HttpFileUploadIntegrationTest.java

@@ -104,6 +104,6 @@ public void onUploadProgress(long uploadedBytes, long totalBytes) {
 
         byte[] downBytes = baos.toByteArray();
 
-        assertArrayEquals(upBytes, downBytes);
+        assertArrayEquals(upBytes, downBytes, "Expected the downloaded bytes to be equal to the uploaded bytes (but they were not).");
     }
 }

smack-integration-test/src/main/java/org/jivesoftware/smackx/iot/IoTControlIntegrationTest.java

@@ -89,7 +89,7 @@ public void processRequest(Jid from, Collection<SetData> setData) throws XMPPErr
 
             SetData data = new SetBoolData(testRunId, true);
             IoTSetResponse response = IoTControlManagerTwo.setUsingIq(conOne.getUser(), data);
-            assertNotNull(response);
+            assertNotNull(response, "Expected " + conOne.getUser() + " to receive an IQ response with an 'setResponse' child element, but no such response was received.");
         }
         finally {
             IoTControlManagerOne.uninstallThing(controlThing);