Use cURL extension instead of Guzzle

[sun]

The only HTTP client operation in this library is to transfer the resulting JSON data in a trivial HTTP POST request with minimal error handling (can't do anything besides outputting the error to CLI).

This PR…

1. Replaces the dependency on Guzzle with a dependency on the PHP cURL extension.
2. Changes ApiClient to return a very simplistic response object.
3. Adjusts TestReporterCommand accordingly.

---

Note: An alternative implementation using PHP HTTP/SSL stream contexts would be possible, but requires additional code to cleanly handle PHP warnings possibly thrown by fopen(), unless error handling is removed altogether.

[pbrisbin]

I'm seeing

PHP Parse error: syntax error, unexpected 'public' (T_PUBLIC) in /home/travis/build/codeclimate/php-test-reporter/src/CodeClimate/Bundle/TestReporterBundle/Command/TestReporterCommand.php on line 86

on Travis.

I think you're missing the closing brace for the switch.

[sun]

Whoops. Sorry.

[sun]

Note that Guzzle is still installed as an (implicit) dependency of satooshi/php-coveralls.

I'm not sure why CodeClimate's reporter depends on coveralls.io's reporter. Unless I'm missing something, all that is being used is the code to convert PHPUnit's Clover XML report into JSON?

[pbrisbin]

I'm not sure why CodeClimate's reporter depends on coveralls.io's reporter... the code to convert PHPUnit's Clover XML report into JSON?

Yes, the dependency is for the XML->JSON class(es). As you might've guessed, none of us here are PHP devs, but we knew that the coveralls package was doing the same basic thing we needed to. One option was to hand-copy the code in and make modifications, the other was to depend on the library and use/extend its classes. We chose the latter.

Any PRs that removed that additional dependency by hand-rolling the XML->JSON stuff would be most welcome.

Also, were you experiencing the certificate issue? If so, do you know if this PR addresses that issue, or should I still make the change in pb-bundled-cert on top of this?

[sun]

the dependency is for the XML->JSON class(es)
…PRs that removed that additional dependency by hand-rolling the XML->JSON stuff would be most welcome.

Would you mind creating a dedicated issue for that, ideally explaining some background information, so others can understand the full rationale of the current code and what the actual server-side technical requirements are?

E.g., one of the first and foremost questions would be why the code coverage data is converted into JSON in the first place, instead of uploading the original/raw Clover XML report?
(separate issue; don't answer here)

were you experiencing the certificate issue? If so, do you know if this PR addresses that issue

Nope, this PR does not help to resolve #3 - it only replaces usage of Guzzle with native cURL (which Guzzle 3 uses anyway under the hood).

The only difference compared to HEAD is that (1) PHP cURL uses the operating system's certs by default and (2) in case of that cert verification error, PHP cURL outputs a more helpful error message.

Regarding the proposed change of pb-bundled-cert, a custom CA cert file can be specified for PHP cURL (via CURLOPT_CAINFO or CURLOPT_CAPATH; see curl_setopt()).

That said, I'd recommend to create a PR for the pb-bundled-cert branch to facilitate a dedicated discussion. (e.g., the cabundle could/should be reduced to the actually required/applicable CA certs only).

[pbrisbin]

Thanks.

I'd recommend to create a PR for the pb-bundled-cert branch to facilitate a dedicated discussion.

My current plan is this:

• Test this branch myself today, hopefully merge it
• Make the bundled-cert change via CURLOPT and open a new PR

[pbrisbin]

@sun can you rebase this over current master? That'll make it so our travis build attempts to submit coverage as another check on the changes before merging.

[pbrisbin]

Testing the branch locally, I'm seeing the following:

% CODECLIMATE_REPO_TOKEN=... php vendor/bin/test-reporter
Unexpected response: 100 Continue
HTTP/1.1 200 OK
Content-Type: text/plain; charset=utf-8
Transfer-Encoding: chunked
Status: 200 OK
Strict-Transport-Security: max-age=31536000
X-Served-By: cc-app01.c45591.bbg
X-Revision: cdc5cd70279a0db12743165cc210897003d00835
X-UA-Compatible: IE=Edge,chrome=1
ETag: "c8fa519244422aeae605c966a97252a2"
Cache-Control: max-age=0, private, must-revalidate
X-Request-Id: fcbb2b9a974f38837d0b11067d43b405
X-Runtime: 0.032205
Date: Wed, 02 Jul 2014 18:29:39 GMT
X-Rack-Cache: invalidate, pass
X-Powered-By: Phusion Passenger
Server: nginx + Phusion Passenger

Test results saved.

[pbrisbin]

@sun is that output expected? Can we hide it? Does there need to be logic to look for 100 response codes too?

[pbrisbin]

I've taken care of the rebase and fixed the incorrect handling of the 100 status.

Opened as a new PR: #12