[sinttest] Assertions to have human readable messages

This adds human-readable text to nearly all assertions in SINT. It is intended that these, together with an XMPP dump of the traffic that was exchanged during the test, allows an observer to have a chance to determine why a particular test failed, without analyzing the test code itself.

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);