-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Make ExceptionMessage constraint strict #3325
Comments
fyi @TomasVotruba |
I would consider this a bugfix, too, and would appreciate a pull request for |
My 2 cents: this makes the (already screwed up) exception api so much more worse... at least add something like: |
imho this is a BC break and must not go to branch 6.5.x-dev right? |
@pscheit You can use explicit annotation for that: https://phpunit.readthedocs.io/en/7.3/annotations.html#expectedexceptionmessageregexp |
I don't want to use annotations and still: i need to write a regexp for "contains". I'm just saying: this issue will decrease developer experience |
I don't use annotations neither, as they're not recommended. There is method: public function test()
{
$this->expectExceptionMessageRegExp('...');
// ...
} But still I prefer specific 100% exception match. Otherwise it has no value to test that exception contains some text. |
I have reverted the change(s) in the |
I decided to deprecate expecting exceptions through annotations in PHPUnit 8 and remove the annotations in PHPUnit 9. |
This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions. |
|
And stale bread is still good for croutons or bread pudding :D |
As I wrote in #3325 (comment), support for expecting exceptions through annotations will be removed in PHPUnit 9 (see #3332 and #3333). I am not sure how to properly address this before the annotation is issue is gone. |
This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions. |
This issue has been automatically closed because it has not had activity since it was marked as stale. Thank you for your contributions. |
Heavily descried in:
#3309 (comment)
Constraint\ExceptionMessage
, used under the hood byexpectExceptionMessage
and@expectedExceptionMessage
is not strict about expected message.Constraint does not check if expected message is same as actual one, but only if it's somewhere present in actual one.
Making those two calls equivalent:
Which is not obvious for developer and actually most of them are not aware about this.
I would like to change behaviour of Constraint to use
===
comparison instead ofstrpos
.For me, personally, it's a bug, yet I can understood that requested change may be considered as bigger one and treated as BC breaker.
If this request will be approved, I'm all in implementing this change myself - either to lowest supported branch, if this would be treated as a bugfix, or later for 8.0 branch (which is not yet existing).
Thanks !
The text was updated successfully, but these errors were encountered: