-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Debug] fix resource type test on HHVM #19116
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
Conversation
xabbuh
commented
Jun 20, 2016
Q | A |
---|---|
Branch? | master |
Bug fix? | yes |
New feature? | no |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #19079 |
License | MIT |
Doc PR |
Do we know why it broke without any change on our side? |
and this simply seems to be the returned value in all HVM versions: https://3v4l.org/D8sLi |
Looks like the tests still do not pass on HHVM. |
0e40d5d
to
e1aad5f
Compare
Tests finally pass (we had to swap the expected and actual arguments for the assertions too). |
@@ -193,6 +193,7 @@ public function flattenDataProvider() | |||
public function testArguments() | |||
{ | |||
$dh = opendir(__DIR__); | |||
$fh = fopen($filename = tempnam(sys_get_temp_dir(), ''), 'r'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$fh = tmpfile();
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense
Tests do now pass again on HHVM. |
Thank you @xabbuh. |
This PR was merged into the 3.2-dev branch. Discussion ---------- [Debug] fix resource type test on HHVM | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #19079 | License | MIT | Doc PR | Commits ------- 2d03ee8 [Debug] fix resource type test on HHVM