[Post-Processors] Replaced ProcessBuilder with Process and add additional tests

[fabianbloching]

Q	A
Branch?	2.0
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#1024
License	MIT

Pull request to support Symfony 4.0

[fabianbloching]

The phpunit 5.7 tests are not running, no tests are executed, see https://travis-ci.org/liip/LiipImagineBundle/jobs/316955919

[sebastianblum]

@dbu David, do you have an idea how to execute the unit tests with PHPUnit 5.7.
Also in other pull requests, not all tests are running, see https://travis-ci.org/liip/LiipImagineBundle/jobs/308329670

[dbu]

i played around in #1027 but still tests are not run. (i think the failure is because i try to cat phpunit.xml which does not exist)

when i run phpunit locally, tests are executed as expected. (i have to use simple-phpunit, otherwise i get the yaml error because phpunit loads a wrong version of the symfony yaml component)

[dbu]

can you please rebase this on the 2.0 branch to get the tests to run?

[dbu]

thanks a lot for this refactoring, and the test setup cleanup! this looks good to me, apart from some tweaking and a question.

one thing that would be great, while we are at cleaning up the build matrix, would be to test the minimum build (something like https://github.com/php-http/HttplugBundle/blob/4c4e25d1812b5ba7868b930946236861c81be473/.travis.yml#L25) to be sure our dependencies in composer.json are correct. thats actually not related to the process builder, so best in a separate PR once we merged this.

[dbu]

i don't think we need to build with symfony 3.4 on more than 1 version of php. 7.2 is enough.

[dbu]

why not $processArguments[] = '--strip-all'; ?

[fabianbloching]

yeah, $processArguments[] = '--strip-all'; makes more sense. I refactored it.

[dbu]

looking at the travis output, i also notice that some of the tests are skipped because of missing flysystem. i think we should have one build where we composer require those dependencies to know that the integration indeed works. (but not add them to composer.json require-dev, otherwise we don't see if other things accidentally depend on flysystem). but again, a task for a separate PR rather than mix it in here.

[sebastianblum]

I created pull request #1035 to extend the build matrix

[dbu]

cool, thanks a lot. looks ready to squash and merge to me!

[fabianbloching]

@robfrawley I rebased against the master and now the merge conflicts are resolved. Maybe you have time to review this pr

[robfrawley]

@fabianbloching FYI, merging the upstream changes into your commit history creates a fragmented, redundant, and "un-clean" looking commit history. I've already fixed this PR, but in the future please try to rebase and not merge upstream changes. Here is a brief example (assuming your remotes are origin (for your own repo) and upstream (for this repo)):

git checkout -b 2.0-upstream upstream/2.0 # checkout upstream (or git pull if you already have it checked out)
git checkout my-feature-branch            # switch back to your feature branch
git rebase 2.0-upstream                   # rebase your feature branch on top of upstream changes

If you encounter any merge conflicts (which can occur while your new commits are being replayed over the upstream changes), manually fix the conflicts and then call git add the-file-with-conflicts.ext followed by git rebase --continue to continue the rebasing. If at any point you want to cancel the rebase, issue git rebase --abort to do so.

[robfrawley]

Thanks @fabianbloching!

[fabianbloching]

@robfrawley Thanks for the detailed briefing.