Skip to content
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.

Sign up for GitHub

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

fix(test): exit code of lime test -- add proof of current behaviour #371

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+598 −4
Open

fix(test): exit code of lime test -- add proof of current behaviour #371

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
38e6200
fix(test): exit code of lime test -- add proof of current behaviour
alquerci Apr 9, 2024
04d0339
fixup! fix(test): exit code of lime test -- add proof of current beha…
alquerci Apr 9, 2024
67168ce
fixup! fix(test): exit code of lime test -- add proof of current beha…
alquerci Apr 10, 2024
77e027d
fixup! fix(test): exit code of lime test -- add proof of current beha…
alquerci Apr 10, 2024
4523808
configure php-cs-fixer
alquerci Apr 10, 2024
54a52f8
execute CS fixer
alquerci Apr 10, 2024
7dccdd1
Allow having pretty method name for test cases
alquerci Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions lib/vendor/lime/lime.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -952,8 +952,7 @@ function lime_shutdown()
);

ob_start();
// see http://trac.symfony-project.org/ticket/5437 for the explanation on the weird "cd" thing
passthru(sprintf('cd & %s %s 2>&1', escapeshellarg($this->php_cli), escapeshellarg($test_file)), $return);
$return = $this->executePhpFile($test_file);
ob_end_clean();
unlink($test_file);

Expand Down Expand Up @@ -1123,6 +1122,20 @@ public function get_failed_files()
{
return isset($this->stats['failed_files']) ? $this->stats['failed_files'] : array();
}

/**
* The command fails if the path to php interpreter contains spaces.
* The only workaround is adding a "nop" command call before the quoted command.
* The weird "cd &".
*
* see http://trac.symfony-project.org/ticket/5437
*/
public function executePhpFile(string $phpFile): int
{
passthru(sprintf('cd & %s %s 2>&1', escapeshellarg($this->php_cli), escapeshellarg($phpFile)), $return);

return $return;
}
}

class lime_coverage extends lime_registration
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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);

passthru(sprintf('cd & %s %s 2>&1', escapeshellarg($this->harness->php_cli), escapeshellarg($tmp_file)), $return);
$return = $this->harness->executePhpFile($tmp_file);
$retval = ob_get_clean();

if (0 != $return) // test exited without success
Expand Down
7 changes: 7 additions & 0 deletions test/unit/vendor/lime/fixtures/failed.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

require_once __DIR__.'/../../../../bootstrap/unit.php';

$test = new lime_test();

$test->is(false, true);
8 changes: 8 additions & 0 deletions test/unit/vendor/lime/fixtures/failed_with_plan_less_than_total.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

require_once __DIR__.'/../../../../bootstrap/unit.php';

$test = new lime_test(1);

$test->is(false, true);
$test->is(true, true);
7 changes: 7 additions & 0 deletions test/unit/vendor/lime/fixtures/failed_with_plan_more_than_total.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

require_once __DIR__.'/../../../../bootstrap/unit.php';

$test = new lime_test(2);

$test->is(false, true);
7 changes: 7 additions & 0 deletions test/unit/vendor/lime/fixtures/pass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

require_once __DIR__.'/../../../../bootstrap/unit.php';

$test = new lime_test();

$test->is(true, true);
13 changes: 13 additions & 0 deletions test/unit/vendor/lime/fixtures/pass_with_one_error.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

require_once __DIR__.'/../../../../bootstrap/unit.php';

error_reporting(E_USER_ERROR);

$test = new lime_test(null, [
'error_reporting' => true,
]);

trigger_error('some user error message', E_USER_ERROR);

$test->is(true, true);
7 changes: 7 additions & 0 deletions test/unit/vendor/lime/fixtures/pass_with_one_parse_error.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

require_once __DIR__.'/../../../../bootstrap/unit.php';

$test = new lime_test();

parse_error
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should be excluded from everywhere that parses php files. At this moment php-cs-fixer, but later phpstan, rector etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, you are right! For the other tools, it was just a reminder to myself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. configure php-cs-fixer
  2. execute CS fixer
  3. Allow having pretty method name for test cases

9 changes: 9 additions & 0 deletions test/unit/vendor/lime/fixtures/pass_with_one_throw_exception.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

require_once __DIR__.'/../../../../bootstrap/unit.php';

$test = new lime_test();

throw new LogicException('some exception message');

$test->is(true, true);
8 changes: 8 additions & 0 deletions test/unit/vendor/lime/fixtures/pass_with_plan_less_than_total.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

require_once __DIR__.'/../../../../bootstrap/unit.php';

$test = new lime_test(1);

$test->is(true, true);
$test->is(true, true);
7 changes: 7 additions & 0 deletions test/unit/vendor/lime/fixtures/pass_with_plan_more_than_total.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

require_once __DIR__.'/../../../../bootstrap/unit.php';

$test = new lime_test(2);

$test->is(true, true);
7 changes: 7 additions & 0 deletions test/unit/vendor/lime/fixtures/skip.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

require_once __DIR__.'/../../../../bootstrap/unit.php';

$test = new lime_test();

$test->skip('some skip message');
7 changes: 7 additions & 0 deletions test/unit/vendor/lime/fixtures/skip_with_plan.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

require_once __DIR__.'/../../../../bootstrap/unit.php';

$test = new lime_test($plan = 2);

$test->skip('some skip message', $plan);
Loading