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.

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

Use symfony/process to run isolated tests #3117

Closed
wants to merge 1 commit into from
Closed

Use symfony/process to run isolated tests #3117

wants to merge 1 commit into from

Conversation

rpkamp
Copy link
Contributor

@rpkamp rpkamp commented May 9, 2018

Fixes #1342

@sebastianbergmann
Copy link
Owner

Thank you for working on this, @rpkamp. However, this will have to wait until #2015 is implemented.

@sebastianbergmann sebastianbergmann added the type/refactoring A refactoring that should be applied to make the code easier to understand and maintain label May 9, 2018
@codecov-io
Copy link

codecov-io commented May 9, 2018

Codecov Report

Merging #3117 into master will increase coverage by 0.33%.
The diff coverage is 92.59%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3117      +/-   ##
============================================
+ Coverage     82.28%   82.61%   +0.33%     
+ Complexity     3487     3468      -19     
============================================
  Files           140      140              
  Lines          9211     9154      -57     
============================================
- Hits           7579     7563      -16     
+ Misses         1632     1591      -41
Impacted Files Coverage Δ Complexity Δ
src/Util/PHP/WindowsPhpProcess.php 0% <ø> (ø) 1 <0> (-3) ⬇️
src/Util/PHP/DefaultPhpProcess.php 55.26% <90.9%> (+11.17%) 15 <0> (-17) ⬇️
src/Util/PHP/AbstractPhpProcess.php 71.91% <93.75%> (+0.28%) 41 <10> (+1) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59d3ce6...2cf5fa6. Read the comment docs.

@rpkamp
Copy link
Contributor Author

rpkamp commented May 9, 2018

That's okay. Tests for windows are still failing anyway. Will look at that later.

@rpkamp
Copy link
Contributor Author

rpkamp commented Aug 26, 2018

I've checked to see if I could fix the errors in AppVeyor and ran into the following:

C:\phpunit\tests\TextUI\phpt-args.phpt:

The problem here is that the argument supplied in ARGS in the .phpt file is passed with a trailing \n, which doesn't seem to matter on *nix, but on windows, the supplied value is literally help\n, which isn't equal to help, so the test fails. I could change the test to if ($argc > 0 && strpos($argv[1], 'help') === 0) and then it would succeed, but it is a break in functionality that others might run into as well. Not really sure how to correctly solve this without any BC breaks. Input would be appreciated.

C:\phpunit\tests\Regression\GitHub\1348.phpt

This one is even tougher, as symfony/process doesn't use regular STDOUT/STDERR on windows as there are several problematic bugs around it. Instead it redirects all OUT/ERR to files. This means that writing to STDOUT from the test file will not be sent to the terminal, but to the file that STDOUT is redirected to instead. This also a BC break that I don't see a nice way of working around. Again, input would be appreciated.

@rpkamp
Copy link
Contributor Author

rpkamp commented Mar 15, 2019

Replaced by #3117

@rpkamp rpkamp closed this Mar 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/refactoring A refactoring that should be applied to make the code easier to understand and maintain
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants