QPID-8350: [Tests][AMQP 1.0] Improve error handling in tests
diff --git a/systests/protocol-tests-amqp-1-0/src/test/java/org/apache/qpid/tests/protocol/v1_0/DecodeErrorTest.java b/systests/protocol-tests-amqp-1-0/src/test/java/org/apache/qpid/tests/protocol/v1_0/DecodeErrorTest.java
index d3d925c..9d189d5 100644
--- a/systests/protocol-tests-amqp-1-0/src/test/java/org/apache/qpid/tests/protocol/v1_0/DecodeErrorTest.java
+++ b/systests/protocol-tests-amqp-1-0/src/test/java/org/apache/qpid/tests/protocol/v1_0/DecodeErrorTest.java
@@ -21,11 +21,11 @@
 package org.apache.qpid.tests.protocol.v1_0;
 
 import static org.apache.qpid.server.protocol.v1_0.type.transport.AmqpError.DECODE_ERROR;
+import static org.apache.qpid.server.protocol.v1_0.type.transport.AmqpError.INVALID_FIELD;
+import static org.apache.qpid.tests.protocol.v1_0.ProtocolAsserts.assertAttachError;
 import static org.hamcrest.CoreMatchers.equalTo;
-import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assume.assumeThat;
 
@@ -40,7 +40,6 @@
 
 import org.apache.qpid.server.bytebuffer.QpidByteBuffer;
 import org.apache.qpid.server.protocol.v1_0.codec.StringWriter;
-import org.apache.qpid.server.protocol.v1_0.type.ErrorCarryingFrameBody;
 import org.apache.qpid.server.protocol.v1_0.type.Symbol;
 import org.apache.qpid.server.protocol.v1_0.type.UnsignedInteger;
 import org.apache.qpid.server.protocol.v1_0.type.messaging.AmqpValue;
@@ -53,12 +52,9 @@
 import org.apache.qpid.server.protocol.v1_0.type.messaging.Target;
 import org.apache.qpid.server.protocol.v1_0.type.transport.Attach;
 import org.apache.qpid.server.protocol.v1_0.type.transport.Begin;
-import org.apache.qpid.server.protocol.v1_0.type.transport.Error;
 import org.apache.qpid.server.protocol.v1_0.type.transport.Flow;
 import org.apache.qpid.server.protocol.v1_0.type.transport.Open;
 import org.apache.qpid.server.protocol.v1_0.type.transport.Role;
-import org.apache.qpid.server.protocol.v1_0.type.transport.SenderSettleMode;
-import org.apache.qpid.tests.protocol.Response;
 import org.apache.qpid.tests.protocol.SpecificationTest;
 import org.apache.qpid.tests.utils.BrokerAdmin;
 import org.apache.qpid.tests.utils.BrokerAdminUsingTestBase;
@@ -71,7 +67,6 @@
     public void setUp()
     {
         _brokerAddress = getBrokerAdmin().getBrokerAddress(BrokerAdmin.PortType.ANONYMOUS_AMQP);
-        getBrokerAdmin().createQueue(BrokerAdmin.TEST_QUEUE_NAME);
     }
 
     @Test
@@ -80,6 +75,7 @@
                           + " Zero or one delivery-annotations, [...]")
     public void illegalMessage() throws Exception
     {
+        getBrokerAdmin().createQueue(BrokerAdmin.TEST_QUEUE_NAME);
         try (FrameTransport transport = new FrameTransport(_brokerAddress).connect())
         {
             final Interaction interaction = transport.newInteraction();
@@ -90,7 +86,6 @@
                        .begin()
                        .consumeResponse(Begin.class)
                        .attachRole(Role.SENDER)
-                       .attachSndSettleMode(SenderSettleMode.SETTLED)
                        .attachTargetAddress(BrokerAdmin.TEST_QUEUE_NAME)
                        .attach()
                        .consumeResponse(Attach.class)
@@ -103,7 +98,10 @@
             {
                 interaction.transferMessageFormat(UnsignedInteger.ZERO)
                            .transferPayload(payload)
-                           .transfer();
+                           .transferSettled(true)
+                           .transferMessageFormat(UnsignedInteger.ZERO)
+                           .transfer()
+                           .sync();
             }
 
             interaction.closeUnconditionally();
@@ -126,25 +124,16 @@
             source.setDynamic(Boolean.TRUE);
             source.setDynamicNodeProperties(Collections.singletonMap(Symbol.valueOf("lifetime-policy"),
                                                                      UnsignedInteger.MAX_VALUE));
-            final Response<?> latestResponse = transport.newInteraction()
+            final Interaction interaction = transport.newInteraction()
                                                         .negotiateProtocol().consumeResponse()
                                                         .open().consumeResponse(Open.class)
                                                         .begin().consumeResponse(Begin.class)
                                                         .attachSource(source)
-                                                        .attachRole(Role.SENDER)
-                                                        .attach().consumeResponse()
-                                                        .closeUnconditionally()
-                                                        .getLatestResponse();
+                                                        .attachRole(Role.RECEIVER)
+                                                        .attach()
+                                                        .sync();
 
-            assertThat(latestResponse, is(notNullValue()));
-            final Object responseBody = latestResponse.getBody();
-            assertThat(responseBody, is(notNullValue()));
-            assertThat(responseBody, instanceOf(ErrorCarryingFrameBody.class));
-
-            final Error error = ((ErrorCarryingFrameBody) responseBody).getError();
-
-            assertThat(error, is(notNullValue()));
-            assertThat(error.getCondition(), is(equalTo(DECODE_ERROR)));
+            assertAttachError(interaction, DECODE_ERROR, INVALID_FIELD);
         }
     }
 
@@ -160,24 +149,15 @@
             target.setDynamic(Boolean.TRUE);
             target.setDynamicNodeProperties(Collections.singletonMap(Symbol.valueOf("supported-dist-modes"),
                                                                      UnsignedInteger.ZERO));
-            final Response<?> latestResponse = transport.newInteraction()
+            final Interaction interaction = transport.newInteraction()
                                                         .negotiateProtocol().consumeResponse()
                                                         .open().consumeResponse(Open.class)
                                                         .begin().consumeResponse(Begin.class)
                                                         .attachTarget(target)
                                                         .attachRole(Role.SENDER)
-                                                        .attach().consumeResponse()
-                                                        .closeUnconditionally()
-                                                        .getLatestResponse();
-
-            assertThat(latestResponse, is(notNullValue()));
-            final Object responseBody = latestResponse.getBody();
-            assertThat(responseBody, is(notNullValue()));
-            assertThat(responseBody, instanceOf(ErrorCarryingFrameBody.class));
-
-            final Error error = ((ErrorCarryingFrameBody) responseBody).getError();
-            assertThat(error, is(notNullValue()));
-            assertThat(error.getCondition(), is(equalTo(DECODE_ERROR)));
+                                                        .attachTargetAddress(BrokerAdmin.TEST_QUEUE_NAME)
+                                                        .attach().sync();
+            assertAttachError(interaction, DECODE_ERROR, INVALID_FIELD);
         }
     }
 
diff --git a/systests/protocol-tests-amqp-1-0/src/test/java/org/apache/qpid/tests/protocol/v1_0/ProtocolAsserts.java b/systests/protocol-tests-amqp-1-0/src/test/java/org/apache/qpid/tests/protocol/v1_0/ProtocolAsserts.java
new file mode 100644
index 0000000..83e9dc7
--- /dev/null
+++ b/systests/protocol-tests-amqp-1-0/src/test/java/org/apache/qpid/tests/protocol/v1_0/ProtocolAsserts.java
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package org.apache.qpid.tests.protocol.v1_0;
+
+import static org.apache.qpid.server.protocol.v1_0.type.transport.AmqpError.NOT_IMPLEMENTED;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.oneOf;
+import static org.junit.Assume.assumeThat;
+
+import org.apache.qpid.server.protocol.v1_0.type.ErrorCarryingFrameBody;
+import org.apache.qpid.server.protocol.v1_0.type.ErrorCondition;
+import org.apache.qpid.server.protocol.v1_0.type.transport.Attach;
+import org.apache.qpid.server.protocol.v1_0.type.transport.Close;
+import org.apache.qpid.server.protocol.v1_0.type.transport.Detach;
+import org.apache.qpid.server.protocol.v1_0.type.transport.End;
+import org.apache.qpid.server.protocol.v1_0.type.transport.Error;
+import org.apache.qpid.tests.protocol.Response;
+
+public class ProtocolAsserts
+{
+    private ProtocolAsserts()
+    {
+    }
+
+    /**
+     * When core spec is not vocal about how the error on attach can be reported,
+     * there are potentially several ways of communicating error back to the client:
+     * <pre>
+     * Attach, Detach(with error)
+     * Attach, Detach, End(with error)
+     * Attach, Detach, End, Close(with error)
+     * End(with error)
+     * End, Close(with error)
+     * </pre>
+     * Thus, in order to assert the error possible codes, we need to get {@link ErrorCarryingFrameBody}
+     * (implemented by {@link Detach}, {@link End}, {@link Close}) and examine error field there.
+     * If error is set, than assert the error code, otherwise, receive subsequent {@link ErrorCarryingFrameBody}
+     * and examine error field there.
+     *
+     * @param interaction interaction
+     * @param expected possible errors
+     */
+    public static void assertAttachError(final Interaction interaction, final ErrorCondition... expected)
+            throws Exception
+    {
+        Response<?> response = interaction.consumeResponse().getLatestResponse();
+        assertThat(response, is(notNullValue()));
+        Object responseBody = response.getBody();
+        assertThat(responseBody, is(notNullValue()));
+
+        if (response.getBody() instanceof Attach)
+        {
+            // expected either Detach or End or Close
+            response = interaction.consumeResponse().getLatestResponse();
+            assertThat(response, is(notNullValue()));
+            responseBody = response.getBody();
+            assertThat(responseBody, is(notNullValue()));
+        }
+
+        assertThat(responseBody, instanceOf(ErrorCarryingFrameBody.class));
+
+        Error error = ((ErrorCarryingFrameBody) responseBody).getError();
+        if (error != null)
+        {
+            assumeThat(error.getCondition(), is(not(NOT_IMPLEMENTED)));
+            assertThat(error.getCondition(), oneOf(expected));
+        }
+        else
+        {
+            // expected either End or Close
+            response = interaction.consumeResponse().getLatestResponse();
+            assertThat(response, is(notNullValue()));
+            Object nextBody = response.getBody();
+            assertThat(nextBody, is(notNullValue()));
+            assertThat(nextBody, instanceOf(ErrorCarryingFrameBody.class));
+            error = ((ErrorCarryingFrameBody) nextBody).getError();
+            if (error != null)
+            {
+                assumeThat(error.getCondition(), is(not(NOT_IMPLEMENTED)));
+                assertThat(error.getCondition(), oneOf(expected));
+            }
+            else
+            {
+                // expected Close
+                response = interaction.consumeResponse().getLatestResponse();
+                assertThat(response, is(notNullValue()));
+                Object body = response.getBody();
+                assertThat(body, is(notNullValue()));
+                assertThat(body, instanceOf(Close.class));
+                error = ((ErrorCarryingFrameBody) body).getError();
+                assertThat(error.getCondition(), is(notNullValue()));
+                assumeThat(error.getCondition(), is(not(NOT_IMPLEMENTED)));
+                assertThat(error.getCondition(), oneOf(expected));
+            }
+        }
+    }
+
+}