Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify assert unreadable in failing cases #58

Open
DleanJeans opened this issue Jun 24, 2021 · 5 comments
Open

Verify assert unreadable in failing cases #58

DleanJeans opened this issue Jun 24, 2021 · 5 comments
Labels
usability Improve user experience with the library

Comments

@DleanJeans
Copy link
Contributor

Code:

assert dpytest.verify().message().contains().content(EXPECTED)

Got:

test_unscramble - AssertionError: assert <discord.ext.test.verify.VerifyMessage object at 0x000001693F0B9AF0>

Expected:

test_unscramble - AssertionError: assert 'apparition' in '`opiaripat` → Not found!'

which is from

assert EXPECTED in dpytest.get_message().content
@itsTheFae
Copy link
Contributor

I'm not sure if I completely understand your issue here.
The new verify builders do make the output a bit more lengthy, but you can still specify a message expression using the standard assert pattern.

assert dpytest.verify().message().contains().content(EXPECTED), f"Missing: {EXPECTED}"
Or whatever message you would like.

@DleanJeans
Copy link
Contributor Author

I was expecting the verify builders to automatically output something useful.

This is just shorter while doing the same thing:

assert EXPECTED in get_message().content

vs

assert verify().message().peek().contains().content(EXPECTED), f'`{EXPECTED}` not in `{get_message().content}`'

@itsTheFae
Copy link
Contributor

I may be wrong, but CraftSpider might be working on something related to setting or cleaning up the assertion errors. There was mention of a similar issue in the discord server at least.

Since the verify builder returns itself, you can take the lengthy verify call chain and assign it to a variable instead of having the whole chain in the assert. It isn't really less typing for you, but it would keep the lines short and more easy to grok at a glance:

test1 = verify().message().peek().contains().content(EXPECTED)
assert test1, f'`{EXPECTED}` not in `{get_message().content}`'

@CraftSpider
Copy link
Owner

Assigning the variable is an option, I'm working on improving that default assert message. Most likely I'll be making the verify stringify/repr output include details about how the assertion failed, making the assert output look like the desired 'expected/got' without any extra work.

@CraftSpider
Copy link
Owner

Note: The new style is about the same number of characters as the old verify_message for the same result, but both are definitely a bit longer than just assert CONTENT in get_message().content. That pattern is perfectly fine if you don't need the full power of the verifiers, they are specifically designed to simplify complex assertion conditions, and to be extensible to even more advanced conditions in the future (EG, allowing multiple content matches, or asserting thing like information about the user that sent it, or whatever people need out of their message assertions).

@DleanJeans DleanJeans reopened this Jun 25, 2021
@CraftSpider CraftSpider added the usability Improve user experience with the library label Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
usability Improve user experience with the library
Projects
None yet
Development

No branches or pull requests

3 participants