Additional tests covering s7 of XEP-0045

[Fishbowler]

Follows #475, #477 and #478. Draft until they're merged.

[evdherberg]

This morning I rebased this branch to Fishbowler:xep-0045-coverage-part3

[evdherberg]

@Flowdalic My rebase may have cleared out your earlier comments, which is unfortunate. I remember one about using enums, probably for error types. Do you remember the other one?

[Flowdalic]

The are still at the commit where I made them (which was "dropped" by you force pushing): https://github.com/igniterealtime/Smack/commit/65dee17f78edd98525fe172adc2bd6c21e2cb56c

[Fishbowler]

Not directly related to this PR, but the following pattern repeats over and over in the MUC integration tests

        EntityBareJid mucAddress = getRandomRoom("smack-inttest-discoinfo");
        MultiUserChat mucAsSeenByOne = mucManagerOne.getMultiUserChat(mucAddress);
        createMuc(mucAsSeenByOne, Resourcepart.from("one-" + randomString));

which suggests that we can add a convenience method to reduce the boilerplate code. I thnk of

public MultiUserChat createMuc(MultiUserChatManager manager, String roomnameComponent) {
…
}

which, for example, determines the nickname prefix (one-, two-) by the provided manager.

I've had a play with this, but we regularly need mucAsSeenByOne later in the test. You could wrap the first two lines and return the MultiUserChat, but you couldn't use it if you needed a mucAsSeenByTwo, or you'd need to get the JID from mucAsSeenByOne, and we'd save no lines and reduce complexity even less.

[Flowdalic]

I've had a play with this, but we regularly need mucAsSeenByOne later in the test. You could wrap the first two lines and return the MultiUserChat, but you couldn't use it if you needed a mucAsSeenByTwo, or you'd need to get the JID from mucAsSeenByOne, and we'd save no lines and reduce complexity even less.

How about?

MultiUserChat mucAsSeenByOne = createMuc(mucManagerOne, "smack-inttest-discoinfo");
MultiUserChat musAsSeenByTwo = getMuc(mucManagerTwo, mucAsSeenByOne);

[guusdk]

Not directly related to this PR, but the following pattern repeats over and over in the MUC integration tests

        EntityBareJid mucAddress = getRandomRoom("smack-inttest-discoinfo");
        MultiUserChat mucAsSeenByOne = mucManagerOne.getMultiUserChat(mucAddress);
        createMuc(mucAsSeenByOne, Resourcepart.from("one-" + randomString));

which suggests that we can add a convenience method to reduce the boilerplate code. I thnk of

public MultiUserChat createMuc(MultiUserChatManager manager, String roomnameComponent) {
…
}

which, for example, determines the nickname prefix (one-, two-) by the provided manager.

I've tried doing this, but it doesn't really work out most of the time. Many of the instance references that are removed by such a method are needed elsewhere in the unit test implementation (for example in the assertions) making the replacement effort rather moot.

[guusdk]

@Flowdalic - is there anything to be done to 'fix' the coveralls check (that's now reporting failure)?

[guusdk]

@Flowdalic - is there anything to be done to 'fix' the coveralls check (that's now reporting failure)?

I've now added a new unit test for the MultiResultSyncPoint util that was added in this PR. Hopefully, that makes the test coverage check happy again.

[guusdk]

Applied review feedback, and squashed ~3 years of commits into one.