fix(test): exit code of lime test -- add proof of current behaviour #371
Conversation
alquerci
force-pushed
the
fix-test-exit-code-of-lime-test-add-proof-of-current-behaviour
branch
from
fc4dbef to
04d0339
Compare
alquerci
changed the title
| @@ -1186,8 +1199,7 @@ public function process($files) | |||
| EOF; | |||
| file_put_contents($tmp_file, $tmp); | |||
| ob_start(); | |||
| // see http://trac.symfony-project.org/ticket/5437 for the explanation on the weird "cd" thing | |||
There was a problem hiding this comment.
[PATCH] lime.php not working on windows when there are spaces in php interpreter path
lime.php, line 541
passthru(sprintf('%s "%s" 2>&1', $this->php_cli, $file), $return);the command fails if the path to php interpreter contains spaces, this obviously prevent all test to run.
In order to work with executables in directories with spaces it should be
passthru(sprintf('"%s" "%s" 2>&1', $this->php_cli, $file), $return);but this fails too on windows: system/exec/passthru all fail when more than two arguments are quoted (it seems a php bug but I couldn't find any reference apart from some comments on the system function page)
In order to work on windows it should be written as
passthru(sprintf('""%s" "%s"" 2>&1', $this->php_cli, $file), $return);this works on windows but still fails on linux (note the double "double quotes": I need to quote the entire command not just the executable and the argument)
The only workaround I could find was something I found on the comments of the system function that is adding a "nop" command call before the quoted command
passthru(sprintf('cd & "%s" "%s" 2>&1', $this->php_cli, $file), $return);|
|
||
| $test = new lime_test(); | ||
|
|
||
| parse_error |
There was a problem hiding this comment.
This file should be excluded from everywhere that parses php files. At this moment php-cs-fixer, but later phpstan, rector etc.
There was a problem hiding this comment.
php-cs-fixer exclude all files that contain "vendor" on its path.
This means that the directory "test/unit/vendor" is ignored.
Fixing that, will requires a structural correction, that is out of scope of this PR.
@connorhu What can I do on the scope of this PR?
There was a problem hiding this comment.
Indeed, you are right! For the other tools, it was just a reminder to myself.
There was a problem hiding this comment.
| $harness = $this->makeHarness(); | ||
|
|
||
| $harness->register([ | ||
| __DIR__."/fixtures/$name.php", |
There was a problem hiding this comment.
| __DIR__."/fixtures/$name.php", | |
| __DIR__.'/fixtures/'.$name.'.php', |
There was a problem hiding this comment.
The $name variable need to be interpreted.
Using single quotes change the behaviour.
There was a problem hiding this comment.
Here, I had think about passing the file path (concrete) or the name (intent).
I choose to pass the intent.
There was a problem hiding this comment.
The action here is to include directory "test/unit/vendor" on php-cs-fixer configuration, then execute the fixer.
There was a problem hiding this comment.
Done in 3 steps
See #371 (comment)
| require_once __DIR__.'/tools/TestCase.php'; | ||
| require_once __DIR__.'/tools/lime_no_colorizer.php'; |
There was a problem hiding this comment.
What is the filename case policy in this directory?
There was a problem hiding this comment.
Same as class name.
@see #301