Decoder and ContentLength plugin

[joelwurtz]

Decoder plugin allows to decode gzip, deflate, compress or chunked body from the response
ContentLengthPlugin "sanitize" the content length header, by setting its value if possible, and if not, (like we are streaming from a process), decorate the body with a chunkstream (so it's compliant with the RFC where no content length header is set)

[dbu]

i would rather skip those tests on hhvm. and have the plugin throw an exception in the constructor if it is incompatible with hhvm

[joelwurtz]

I'm more in the thinking of providing no support for hhvm, i really dont know anyone who is using it, and with upcoming php7 it may be even worse.
IMO I don't want to pass too many hours supporting something not used at all (and rather use them to write more things for php-http :) ).

[sagikazarmark]

I would say HHVM is not a top priority for now, but it would be nice to support it eventually.

[dbu]

i have no worries making the tests skip things around streams + hhvm. but i would like to keep the hhvm build green. at liip we do use it in some projects, for example. skipping tests should not be a big hassle. if we end up with most things not working on hhvm, we can reconsider.

[dbu]

this is really cool! do clients like guzzle re-implement all that inside their code? on the other hand, what will guzzle do itself in the end? not much?

[joelwurtz]

Guzzle doesn't implement this, but i think curl does (only for the transfer encoding header if my memory is good)

[joelwurtz]

Will work on hhvm implementation, aside from this, any other remarks ? or is this good to merge ?

[sagikazarmark]

I haven't really followed this thread, so ask @dbu for any remarks. 👍 from me, awesome work @joelwurtz as usual.

[dbu]

looks good to me. i suggest that you simply add something to the tests to skip. if it would be phpunit, it would be

public function setUp()
{
     if (defined('HHVM_VERSION')) {
         $this->markTestSkipped();
    }
}

[sagikazarmark]

IIRC Akeneo has a SkipExtension for PHPSpec.

[dbu]

if its too much hassle, i can add a conditional `rm` of the tests in the .travis.yml files to simply remove the tests when running with hhvm.

[sagikazarmark]

if its too much hassle

AFAIK it uses the same logic/annotations as phpunit does. How is it too much hassle?

However I checked it and it can only check existing classes/interfaces, not the PHP env.

[dbu]

However I checked it and it can only check existing classes/interfaces, not the PHP env.

thats what i meant with too much hassle. if we need to find some phpspec plugins and whatnot, we can just as well do it from .travis.yml. as we agree hhvm is not top priority, we should have a simple way to skip those tests.

[sagikazarmark]

I would still stick to the setUp/let method instead of some weird rm logic. 😜

[sagikazarmark]

use PhpSpec\Exception\Example\SkippingException;
//...
function let()
{
    if(defined('HHVM_VERSION')) {
        throw new SkippingException();
    }
}

[dbu]

ah, if that works then sure lets do that. why is that namespace "Example"?

[sagikazarmark]

Because tests are called "examples" in phpspec terminology:

• There are Suites which are collections of tests (for example you have one spec/ directory)
• There are Specifications which are collections of examples (one class, several test methods)
• There are Examples which are the actual test methods (eg. it_does_something)

[joelwurtz]

Ok on hhvm, ready to merge ?

[dbu]

great. can you please move hhvm out of the allowed to fail tests again? then we are good to merge.

we have to remember to mention in the plugin doc that the encoding plugin does not support hhvm.

[dbu]

actually, maybe you can mark the stream library as conflicting with hhvm? we have no hard dependencies on the libraries used by the plugins, so people trying to run this will notice there is a problem even if they don't read the documentation.

[sagikazarmark]

Conflict means uninstallable, right?

[dbu]

it means composer require / install will fail when running under hhvm, yes. i am not suggesting to make plugin conflict with hhvm, only the stream library joel wrote

[joelwurtz]

test will failed then on hhvm for this, as we require this lib in dev packages, any way of avoiding that ?

[dbu]

oh well. i guess that won't work. lets leave it like this then. just move hhvm out of allowed failures and then we merge this.

we could remove it from require-dev and have travis run composer require if not on hhvm, but that is complicated. i guess hhvm users are used to having to check whether libraries work on hhvm...

[joelwurtz]

Encoding library can work on hhvm, just requiring implementing many filters from the php world :/

[dbu]

👍