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

Allow setting custom failure messages #17

Open
wouterj opened this issue Jan 5, 2021 · 4 comments
Open

Allow setting custom failure messages #17

wouterj opened this issue Jan 5, 2021 · 4 comments

Comments

@wouterj
Copy link
Contributor

wouterj commented Jan 5, 2021

PHPUnit's asserts (and all asserts provided by Symfony) allow you to specify a custom failure message as last argument. This gives you the possibility to add some more context to an otherwise vague error.

Would it be an idea to also add this feature to Browser?

@kbond
Copy link
Member

kbond commented Jan 5, 2021

I thought about this but decided against for the following reasons:

  1. Dusk doesn't provide this option (see [Feature Request] Assert Message laravel/dusk#206)
  2. Some assertions make multiple "sub" assertions - should I use the message to override all these?
  3. Removes the possibility of adding extra optional method arguments in the future
  4. I don't think I've ever used custom messages in phpunit

This all being said I don't have a strong opinion on this.

@benr77
Copy link
Contributor

benr77 commented Jan 10, 2023

I also rarely use this feature with PHPUnit, but in some cases it's very handy to help my future self.

I understand why you've decided against, but is there another way to output a message, perhaps with a method that follows the assertion? Something like:

->assertSee('23rd January 2023')
->reportOnFailure('The date of the reservation is not correct')

It's just an idea and it's not the end of the world if we can't do this - just have to add a comment in the test source code and then go look that up on failure so no drama really.

@kbond
Copy link
Member

kbond commented Jan 10, 2023

I'm fine adding extra optional messages to the assertion methods.

Some assertions make multiple "sub" assertions - should I use the message to override all these?

Still not sure how we'd handle these.

Removes the possibility of adding extra optional method arguments in the future

The api is pretty stable so this isn't much of a concern anymore.

perhaps with a method that follows the assertion

That would be tough because the assertion happens in the method above. Do you have an idea on how this could work??

@benr77
Copy link
Contributor

benr77 commented Jan 10, 2023

That would be tough because the assertion happens in the method above. Do you have an idea on how this could work??

Not at all it was just a throwaway suggestion :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants