Adds --passthru and --passthru-php options. Adds --verbose option. #361
Conversation
…lite is missing. Enables splitting --testsuite option by ,. GH issue: paratestphp#347
…cu). Adds composer script info to README.
…dates readme. GH issues: paratestphp#354, paratestphp#360
| * | ||
| * @return $this | ||
| */ | ||
| public function run(string $binary, array $options = [], array $environmentVariables = []) | ||
| public function run(string $binary, array $options = [], array $environmentVariables = [], string $passthru = null, string $passthruPhp = null) |
There was a problem hiding this comment.
Imo, the param type declaration needs to be ?string, otherwise it wouldn't be possible to pass (theoretically, when method is called in PHP code) null as $passthru and then set $passthruPhp.
There was a problem hiding this comment.
The ? prefix was introduced in 7.1 (7.0 didn't have nullable types). We were stuck on 7.0 for quite some time so I'm still used to doing it like that ;)
But I just checked the composer.json of paratest and the PHP version is >= 7.1 so I'll fix that.
| * | ||
| * @return string | ||
| */ | ||
| protected function getFullCommandlineString($binary, $options, string $passthru = null, string $passthruPhp = null) |
There was a problem hiding this comment.
Same here, I think param type needs to be ?string for $passthru and $passthruPhp.
| * @param array $options command line options | ||
| * @param string $binary executable binary name | ||
| * @param array $options command line options | ||
| * @param null $passthru |
There was a problem hiding this comment.
I guess this should be string|null?
| { | ||
| // The order we add stuff into $arguments is important | ||
| $arguments = [$binary]; | ||
| // Note: | ||
| // the arguments MUST come last and we need to "somehow" | ||
| // mere the passthru string in there. |
There was a problem hiding this comment.
Typo? Should be "merge", right?
| // Note: | ||
| // '--log-junit' '/tmp/PT_LKnfzA' | ||
| // is appended by default where PT_LKnfzA is randomly generated - so we remove it from the resulting command | ||
| $command = preg_replace("# '--log-junit' '[^']+?'#", '', $command); |
There was a problem hiding this comment.
OK nice, very strict way to test the result. 👍 (Personally I guess I would have just used assertContains here.)
There was a problem hiding this comment.
I would actually like to make the '--log-junit' '/tmp/PT_LKnfzA part configurable in ExecutableTest but that would probably be a bigger refactoring :)
As for "strictness": I try to avoid "contains" as it opens up the assertion for "unwanted" side effects + in this case I would need two assertions instead of one.
|
If this gets merged I would add a small write-up to the docu on how to get code code coverage running with paratest:
|
|
Thanks a lot @paslandau for the PR, perfect now! Got merged. Better docs would be awesome as I can see some devs having problems how to get it properly set up. If you plan further PRs with new features, please let me know, otherwise I would tag a new version soon (maybe after the docs write-up?). |
|
@andreasschroth docs done => #362 located under docs/code-coverage.md Should be the last one for now - would appreciate a new version! /edit |
|
@paslandau Thank you! Awesome work. I just tagged Version 2.2.0, so everyone can make use of the latest improvements. |
Fixes #360 and #354