Moved TEST_COMMAND to global section #203
Conversation
I did also add php 7.2
|
To see if dependencies have an incoming release with PHP 7.2, it would make sense to use PHP 7.2 here: Lines 29 to 30 in 766687e |
|
Do you plan to solve failing tests in this PR? |
fbourigault
force-pushed
the
patch-global
branch
from
fd9ddb0 to
81efd33
Compare
fbourigault
force-pushed
the
patch-global
branch
from
fcf1ad0 to
8ad73b5
Compare
|
🎉 HttplugBundle is now successfully tested against PHP 7.2
|
| @@ -63,14 +64,15 @@ public function testProfilingWithCachePlugin() | |||
| $this->assertEquals('example.com', $stack->getRequestHost()); | |||
| } | |||
|
|
|||
| /** | |||
| * @expectedException \Exception | |||
There was a problem hiding this comment.
i discussed once with sebastian bergman and he is saying he agrees that the annotation is less clean than calling setExpectedException. the reason is that if the exception is thrown too early, the test would wrongly be green. this is particularly true when we expect such a generic exception
There was a problem hiding this comment.
So, it's better to have a if/else at the right position in the test?
There was a problem hiding this comment.
why if/else? afaik the $this->setExpectedException call is the best practice way. this makes sure the exception is only considered correct when thrown after that line in the code, and you have namespace resolution.
There was a problem hiding this comment.
setExpectedException was removed in 6.0 and replaced by expectException. That's why I moved to annotation.
There was a problem hiding this comment.
Yeah, We need the annotation for BC purpose...
There was a problem hiding this comment.
We can keep this as-is and switch to expectException in next-major.
| @@ -49,7 +50,7 @@ | |||
| "symfony/browser-kit": "^2.8 || ^3.0", | |||
| "symfony/dom-crawler": "^2.8 || ^3.0", | |||
| "polishsymfonycommunity/symfony-mocker-container": "^1.0", | |||
| "matthiasnoback/symfony-dependency-injection-test": "^1.0", | |||
| "matthiasnoback/symfony-dependency-injection-test": "^1.0 || ^2.0", | |||
There was a problem hiding this comment.
do we need both versions to support the range of php versions we support?
There was a problem hiding this comment.
Yes, we can't install PHPUnit 6.0 because of a conflict on sebastian/exporter.
|
Thank you. I'm happy to merge this. |
|
What do you think @dbu? |
|
Thank you all! |
I did also add php 7.2