[Post-Processors] Add timeout option, refactor processors, add tests for all, add/remove/deprecate options

[robfrawley]

Q	A
Branch?	1.0
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	yes
Tests pass?	yes
Fixed tickets	#857
License	MIT
Doc PR	(docs included in PR)

This is a large pull request, and it is still a WIP, so I'm not going to go through everything just yet, but for the time being, here is an initial list of changes introduced:

• Enable configuring timeout, prefix, env, and options on the ProcessBuilder per-post-processor config entry.
• Major refactoring of post-processors to both cleanup their implementations and enable easier writing of tests for them:
  • Deprecated ConfigurablePostProcessorInterface and it's processWithConfiguration() method. Moving forward, usage of PostProcessorInterface::process() without a second parameter (containing an array of options) will throw a deprecation message advising that in 2.0 the second parameter is required. We shouldn't need two interfaces for post processors: this should allow us to migrate to one for 2.0.
  • Pull a bunch of common functionality into a new AbstractPostProcessor that all our core-post-processors inherit from.
  • Deprecate all setter methods on post-processes: their state should be set via the constructor and their per-usage behavior overridden via the second argument of process(). These setters provided no value whatsoever and simply expose an additional public method of our API that we cannot change without a major version release. (We should find more instances of useless methods that we can deprecate for removal or change of visibility to protected/private in 2.0).
  • Changes to some of the specific options available via the post-processors (all changes simply throw deprecation warnings and all prior options continue to work as expected until 2.0).
  • Additions of a bunch of post-processor options that were previously unavailable (see changes to the RST documentation for reference).
• Due to the refactoring, new tests have been added for all post-processors, bringing their code coverage up from 0% to 100%.

There are other changes, but those are the broad strokes that should help open an initial discussion.

[alexwilson]

Very happy to see this as it's been sitting at the top of my ImagineBundle "todo" list for a loooooong time... 🙇

[robfrawley]

@antoligy Glad to hear you're onboard with the goals of this PR! 👍 I finally got around to updating the code here to support PHP 5.3 (I always forget I can't rely on things like closures being bound to their current object scope, or short array syntax, etc).

Anyway, tests now pass and Coveralls shows this PR increases code coverage 6.9% to an overall value of 82.4% coverage globally for our codebase, with the post-processors now 100% covered!

See: https://coveralls.io/jobs/29359232

A brief, initial review with any thoughts, comments, or concerns would be welcome from anyone with the time to do so.

[lsmith77]

maybe this should go into 2.0 instead now?

also note that ProcessBuild is dropped in Symfony 4
#1025

[robfrawley]

I think that's an excellent idea; this also relates to the issues we were having with accepting #931. This is old now and was only 90% done at the time, but I'll take some time to go through everything and clean it up shortly.

[maximgubar]

@robfrawley is this PR still relevant ?

[robfrawley]

@maximgubar Yes; I have a local copy that targets 2.0 that should be done soon. I'll update here once it's finished.

[FabianSchmick]

Any updates on this? Because I got the same issues as mentioned here #987 and #857

[franmomu]

This can be close because of with #1219

[dbu]

finished in #1219, thanks @robfrawley and @franmomu !