upgrade tests to new exception syntax

[TomasVotruba]

No description provided.

[TomasVotruba]

Why was there "variadic", a typo?

[keradus]

no no no no no

this annotation supports one and two parameters, one of them is exception class, second, optional, is exception message.

this docblock is equivalent to

/**
 * @expectedException \Exception 
 * @expectedExceptionMessage variadic
 * @expectedExceptionMessageRegExp /^A variadic \w+ message/
 */

[TomasVotruba]

I see, thanks for sharing. Is there somewhere documented?

So correct replacement would be this?

$this->expectException('Exception');
$this->expectExceptionMessage('variadic');

[keradus]

no, you shall still respect expectedExceptionMessageRegExp
expectedExceptionMessage is not causing === "variadic", but /.*variadic.*/ under the hood

probably it is, but i was not using documentation for so long, i won't point it

[TomasVotruba]

In the comment above you mix various rules then. So it's like this?

$this->expectException('Exception');
$this->expectExceptionMessageRegExp('#variadic#');

[keradus]

@sebastianbergmann , I believe that I can elaborate on my answer, so maybe you won't need to spend time on it. Of course, if you want to step in, do it ;)

[keradus]

I'm sad you don't want to believe me @TomasVotruba nor check it on your own. I'm using this great project for so long and that intensive, I got chance to understood it and know few aspects of it that can be considered tricky, like the one you faced here ;)

To sum up:

1. one can apply that diff, both versions, before and after patch, mean the same

 /**
- * @expectedException \Exception variadic
+ * @expectedException \Exception
+ * @expectedExceptionMessage variadic
  * @expectedExceptionMessageRegExp /^A variadic \w+ message/
  */

2. both, message and messageRegExp expectations are respected and executed
3. message foo is in fact .*foo.* regex (but you can't provide multiple regexes annotations!) (disclaimer - yes, I would prefer personally if that would not happen, yet it does)
4. to replace annotations to code, we would have:

$this->expectException('Exception');
$this->expectExceptionMessage('variadic');
$this->expectExceptionMessageRegExp('/^A variadic \w+ message/');

[keradus]

As I said, I don't rely on documentation, yet that shall be better - I took 5 minutes and grab some links for you, @TomasVotruba . I hope those will help you to understood how it works, if my elaboration is not enough.

expectedException annotation regex is defined here:

phpunit/src/Util/Test.php

Line 65 in 952160e

private const REGEX_EXPECTED_EXCEPTION = '(@expectedException\s+([:.\w\\\\x7f-\xff]+)(?:[\t ]+(\S*))?(?:[\t ]+(\S*))?\s*$)m';

As you can see, it can defined class, message and code, eg message is handled here:

phpunit/src/Util/Test.php

Line 365 in 952160e

$message = \trim($matches[2]);

Ten, util is used in TestCase to set expectations:

phpunit/src/Framework/TestCase.php

Line 1606 in 64d377c

$this->expectExceptionMessage($expectedException['message']);

And asserted here

phpunit/src/Framework/TestCase.php

Line 1169 in 64d377c

$this->expectedExceptionMessage

by creating ExceptionMessage constraint.

So finally, we can see how message we provided in annotation is comparad to message from exception that happened:

phpunit/src/Framework/Constraint/ExceptionMessage.php

Line 47 in 2e9ab30

return \strpos($other->getMessage(), $this->expectedMessage) !== false;

As we see, it's not ===

[TomasVotruba]

This feature is WTF to me. I was confused by your answer and wanted to know for sure.

Thanks for the code, that's what I needed to be able to implement it correctly.

[keradus]

as I said, there are some quirks and tricky parts, yet for most use cases they do the job and normally, person doesn't need to think about it.

glad to help ;)

[sebastianbergmann]

Are you sure that none of these tests in fact test that the annotation for expecting exceptions works?

[TomasVotruba]

There should be some I guess. It's not clear from class/method naming to me.
Maybe have "ExpectedExceptionAnnotationTest" with explicit comment?

[keradus]

without having explicit tests, I would suggest to not drop those annotations for now

[TomasVotruba]

@sebastianbergmann What do you think about explicit test case names?

[TomasVotruba]

Closing as unclear what to do to close it and not such a big deal atm.

@keradus Thanks for examples again.