Fix set content headers#218
Fix set content headers#218sanpii merged 4 commits intoBehatch:masterfrom razbakov:217-content-type-header
Conversation
|
Please, make one PR per fonctionnality/fix. Moreover, it’s not a good idea to depends on an extrenal service for the test. Thank you. |
| - 7.0 | ||
| - 7.1 | ||
| - nightly | ||
| - hhvm |
There was a problem hiding this comment.
Instead of removing ci-tests on hhvm we should actually follow the advice of travis:
HHVM is no longer supported on Ubuntu Precise. Please consider using Trusty with `dist: trusty`.
| $expected = str_replace('\\"', '"', $expected); | ||
| $actual = $this->request->getContent(); | ||
| $message = "The string '$expected' is not equal to the response of the current page"; | ||
| $message = "Response is: $actual"; |
There was a problem hiding this comment.
Now we don't see the $expected anymore in the message.
Maybe something like "The response of the current page is not '$expected', it is: $actual"
There was a problem hiding this comment.
You see expected always right before the error message.
There was a problem hiding this comment.
That depends on your output formt. With the format (-f) pretty (which is default) you see the expected before the error message, but not with e.g. progress or PhpStormBehatFormatter.
There was a problem hiding this comment.
What about the following methods?
It make more sense to see $actual in error message, rather than $expected
There was a problem hiding this comment.
Yes, the $actual should be in the message. But in my opinion, also the $expected to make life easier when something fails.
| "The header '$name' doesn't contain '$value'" | ||
| $actual = $this->request->getHttpHeader($name); | ||
| $this->assertContains($value, $actual, | ||
| "The header '$name' value is '$actual'" |
There was a problem hiding this comment.
Same as above, now missing the expected value.
Maybe something like "The header '$name' doesn't contain '$value', it is: $actual"
|
also wait for this merge 👍 |
CONTENT_* are not prefixed with HTTP_ in PHP when building $_SERVER Code taken from Behat\Mink\Driver\BrowserKitDriver::setRequestHeader Closes #217
in order to test Goutte Client
|
@mitalcoi @elkangaroo @sanpii please review again, I made suggested changes. |
| public function theHeaderShouldBeContains($name, $value) | ||
| { | ||
| trigger_error( | ||
| sprintf('The %s function is deprecated since version 3.0 and will be removed in 3.1. Use the %s::theHeaderShouldContain function instead.', __METHOD__, __CLASS__), |
There was a problem hiding this comment.
Deprecated since 3.1 and removed n 4.0 instead.
There was a problem hiding this comment.
fixed to 3.1 and 4.0
- fix function name theHeaderShouldContain (was theHeaderShouldBeContains) - add $actual to assert exception to simplify debugging
|
any idea why travis failed? |
|
@OskarStark it looks like a problem with selenium. Maybe a chromium update? |
|
Could you restart the build? I tried the change manually in my project and it works. If you need help, you can add me as a collaborator to this project, I'm still member of the Sonata-Project ;-) |
|
Closes #212 |
|
any news here? Is this PR (which is not release yet) maybe related to the travis problem? I had the same issue and if I apply the change manually it worked for me 👍 |
|
@sanpii https://github.com/Behat/MinkExtension/releases/tag/2.3.1 is released and should fix this error. Can you please restart the travis job? |
|
@OskarStark I relaunched the test, they still fails… Help welcome: #242 |
5 participants
CONTENT_* are not prefixed with HTTP_ in PHP when building $_SERVER
Code taken from Behat\Mink\Driver\BrowserKitDriver::setRequestHeader
Closes #217