assert_message_x should operate on a message passed as a parameter

[JosephFKnight]

I'm shocked it took me this long to notice this. But the behavior of (almost, excepting the one I wrote) all of the assert_message_x functions is not intuitive. Instead of waiting for a reply, these functions should be passed a message as an argument. This will allow finer control over which messages get tested, and I believe will reduce unexpected behavior when messages are flying back and forth between the bots. we should make this change in feature/add_more_tests. I will gladly do it, but I don't want to step all over your branch without your permission.

[Cobular]

Oh yeah, that should be fixed. Do you want the message argument to be passed into the wait_for() check? That seems like the most convenient way to do it, unless you want to construct an instance of the Message class, which i'm not sure how to do.
The other option is to make a message_properties class (or dict, but class w/ optional inputs would be easier to use) that stores things like content, author, reactions, role, or whatever else we can think of that could be represented as a string. This could be passed into the check and let the user make whatever filters they want pretty easily.

EDIT: Are you suggesting that we move some of the filter logic from the general test function into the check, so that the wrong messages aren't processed by the check? In that case, there is already a bit of that (the author and channel are checked), but it doesn't do what you want. On the other hand, changing checks this way will probably mean all failures come out as timeout errors rather than the more specific errors they give currently. Back on the first hand, we don't really do much with the specific errors that DXsmiley made anyway, so it really isn't a loss at all to change that.

[Cobular]

Honestly, after reading through the tests, they seem to be a bit of a mess, with confusing purposes and a lot of weirdness brought about by the custom errors and the fact that I'm pretty sure that the replies assert_reply family doesn't actually use the assert_reply family at all. I think, after we finish writing examples for these tests (#6) and finishing up the rest of the 1.0 milestone, we re-do the majority of the way the tests work on the inside (and possibly on the outside too) before 2.0, since that will be a pretty decent chunk of work and will require some actual planning to make sure everything is as sensible and clean as possible.

[JosephFKnight]

No, you may be overthinking it. I'll use assert_message_equals as an example. Here's how it looks right now:

async def assert_message_equals(self, matches):
    """ Waits for the next message.
        If the message does not match a string exactly, fail the test.
    """
    response = await self.wait_for_message()
    if response.content != matches:
        raise ResponseDidNotMatchError
    return response

And here is how it should look:

async def assert_message_equals(self, message, matches):
    if message.content != matches:
        raise ResponseDidNotMatchError
    return response

This applies the Single Responsibility Principle to the functions (whereas before they had two responsibilities: waiting for a message and asserting its contents) and reduces unwanted side-effects (like the Discord API serving the wrong message when messages are moving too fast). This will also enable us to write the complementary assert_reply_equals function thusly:

async def assert_reply_equals(self, contents, matches):
    message = await self.wait_for_reply(contents)
    return self.assert_message_equals(message, matches)

Thus greatly reducing duplicate code and enhancing readability.

[JosephFKnight]

@JakeCover This is not a huge undertaking, rather a small refactor. Give me a few hours this afternoon and I'll have the interface clean as a whistle. Cleaning code is my favorite pastime.

[Cobular]

Ohhh ok, I see what you mean. Definitely go ahead, just plz make sure to write descriptive docstrings, even if it seems self-explanatory!

Cleaning code is my favorite pastime.

Good good, there's plenty of that to be done around here lol

[JosephFKnight]

yessir boss man

[JosephFKnight]

Done. Commits pushed to #20. checkout how much cleaner it is.

I also removed several extraneous comments (comments that restate the obvious are not clean code!) and rewrote a few docstrings (Python docstrings should be in the present tense. As though you are giving the program an imperative. ie "Do this" instead of "Does this").

Closing this issue.