ARTEMIS-2488: Handle the case where source address is null

[brusdev]

(cherry picked from commit 2ec96bf)

downstream: ENTMQBR-3258

[jirkadanek]

Are there any practices about organizing the more "mock heavy" tests? For me, this order of things makes a bit more sense:

    @Test(expected = ActiveMQAMQPNotFoundException.class)
    public void testAcceptsNullSourceAddressWhenInitialising() throws Exception {
        Source source = new Source();
        source.setAddress(null);

        Sender mockSender = mock(Sender.class);
        when(mockSender.getRemoteSource()).thenReturn(source);

        AddressQueryResult queryResult = new AddressQueryResult(null, Collections.emptySet(), 0, false, false, false, false, 0);

        AMQPSessionCallback mockSessionCallback = mock(AMQPSessionCallback.class);
        when(mockSessionCallback.addressQuery(any(), any(), anyBoolean())).thenReturn(queryResult);

        AMQPConnectionContext mockConnContext = mock(AMQPConnectionContext.class);

        ProtonServerSenderContext sc = new ProtonServerSenderContext(mockConnContext, mockSender, null, mockSessionCallback);

        sc.initialise();
    }

[jirkadanek]

You are mocking too much, IMO. The first null parameter to addressQuery is supposed to be coming in from calling SimpleString.toSimpleString(source.getAddress()); but here you hardcode the null in queryResult for whatever value went into calling that addressQuery.

I'd add assert that addressQuery got called with null, or configure the when( to only respond with this result for null as first parameter, or maybe try to avoid mocks as much as possible and use real server instead of mockSessionCallback or just make the test assert that mockSessionCallback.addressQuery got called with null as first parameter.

That initialize() thing is soooo long.

[jirkadanek]

@lulf It's probably just me, but I'd still want to just try connecting to a full broker to check if this is fixed. I somehow cannot get myself to trust the mock-rich test above. Especially after some time passes, in future releases.

[lulf]

@jirkadanek I don't disagree, I'd love to not having to mock that much. However, triggering this from an integration test might be hard, we originally triggered this via an Artemis plugin. Also, my knowledge of the test framework used in Artemis is limited, and this seemed the (believe it or not :) ) the simplest way to write a test for this.

[brusdev]

Merged it