Revert https://github.com/bigcommerce/bigcommerce-api-php/pull/155 #174

bc-ruth · 2016-06-10T18:07:22Z

…e away, resetting to default

What:
Reverting #155
and optionally define STDIN to address issues mentioned in same PR ^

Why:
Breaking basic tests
curl_exec(): CURLOPT_INFILE resource has gone away, resetting to default
FAILURES!
Tests: 70, Assertions: 476, Errors: 12.

Testing:
................................................................. 65 / 70 ( 92%)
.....
OK (70 tests, 541 assertions)

coveralls · 2016-06-10T18:11:37Z

Coverage decreased (-0.9%) to 64.717% when pulling 6f164a5 on bc-ruth:OMNI-1167 into 345d27d on bigcommerce:master.

coveralls · 2016-06-10T18:31:19Z

Coverage decreased (-1.1%) to 64.466% when pulling 40c0972 on bc-ruth:OMNI-1167 into 345d27d on bigcommerce:master.

lord2800 · 2016-06-10T18:34:33Z

I'm... not sure if this is the correct way to fix this. It needs some more investigation I think.

coveralls · 2016-06-10T21:16:58Z

Coverage decreased (-0.4%) to 65.226% when pulling f773ef5 on bc-ruth:OMNI-1167 into 345d27d on bigcommerce:master.

coveralls · 2016-06-12T04:59:21Z

Coverage decreased (-0.3%) to 65.294% when pulling 407a278 on bc-ruth:OMNI-1167 into 345d27d on bigcommerce:master.

PascalZajac · 2016-06-12T08:14:12Z

This isn't a straight revert though; you're adding new behaviour for every request by changing the flag after the connection is closed (which tbh I don't understand anyway).

I'm also confused why the tests would pass on the original PR, I guess we don't have the flag to treat warnings as errors?

bc-ruth · 2016-06-12T18:43:33Z

src/Bigcommerce/Api/Connection.php

@@ -91,6 +91,9 @@ class Connection
     */
    public function __construct()
    {
+        if (!defined('STDIN')) {
+            define('STDIN', fopen('php://stdin', 'r'));
+        }
        $this->curl = curl_init();


@PascalZajac I can remove this change to make this PR a proper revert.
I was hoping to resolve the issue(s) reported in #139

bc-ruth · 2016-06-12T18:47:18Z

@PascalZajac the tests were not passing on the original PR. The regression wasn't caught by our basic tests then because we updated the bigcommerce-api-php repo but did not push/bump a new version in bcapp.

aleachjr · 2016-06-12T19:43:59Z

The thing to keep in mind here is that this is the public API and its test suite was passing. We use this in functional testing but our first priority should be to stabilize it for the public use.

PascalZajac · 2016-06-13T05:11:41Z

I understand now, thank you for clarifing @bc-ruth. In that case, I'm happy for us to accept Ruth's PR, since it does seem to be the "accepted" way to define the constant when not running in CLI mode.

bc-ruth · 2016-06-13T18:44:25Z

@Lewiscowles1986 can you pull this branch and verify that this fixes your issue in #155 ?

Lewiscowles1986 · 2016-06-13T19:50:41Z

Hi all.

I'm not encountering any breakage in test suites; would it be possible to let me know what is wrong? I've read, but I'm not sure I get it.

I've just run

git clone https://github.com/bigcommerce/bigcommerce-api-php
cd bigcommerce-api-php
composer install
phpunit

here is the output

lewis@lewis-HP-ENVY-17-Notebook-PC:~/projects/php$ git clone https://github.com/bigcommerce/bigcommerce-api-php
Cloning into 'bigcommerce-api-php'...
remote: Counting objects: 1543, done.
remote: Total 1543 (delta 0), reused 0 (delta 0), pack-reused 1543
Receiving objects: 100% (1543/1543), 422.07 KiB | 226.00 KiB/s, done.
Resolving deltas: 100% (702/702), done.
Checking connectivity... done.
lewis@lewis-HP-ENVY-17-Notebook-PC:~/projects/php$ cd bigcommerce-api-php/
lewis@lewis-HP-ENVY-17-Notebook-PC:~/projects/php/bigcommerce-api-php$ ls
composer.json  LICENSE  phpunit.xml.dist  README.md  src  test
lewis@lewis-HP-ENVY-17-Notebook-PC:~/projects/php/bigcommerce-api-php$ composer install
Loading composer repositories with package information
Updating dependencies (including require-dev)
  - Installing symfony/yaml (v2.8.7)
    Downloading: 100%         

  - Installing symfony/stopwatch (v3.1.0)
    Downloading: 100%         

  - Installing symfony/polyfill-mbstring (v1.2.0)
    Loading from cache

  - Installing symfony/console (v3.1.0)
    Downloading: 100%         

  - Installing symfony/filesystem (v3.1.0)
    Downloading: 100%         

  - Installing symfony/config (v3.1.0)
    Downloading: 100%         

  - Installing psr/log (1.0.0)
    Loading from cache

  - Installing symfony/event-dispatcher (v2.8.7)
    Downloading: 100%         

  - Installing guzzle/guzzle (v3.9.3)
    Downloading: 100%         

  - Installing satooshi/php-coveralls (v1.0.0)
    Downloading: 100%         

  - Installing codeless/logmore (v0.9.0)
    Downloading: 100%         

  - Installing nikic/php-parser (v2.1.0)
    Loading from cache

  - Installing codeless/jugglecode (v1.0.0)
    Downloading: 100%         

  - Installing sebastian/version (1.0.6)
    Loading from cache

  - Installing sebastian/global-state (1.1.1)
    Loading from cache

  - Installing sebastian/recursion-context (1.0.2)
    Loading from cache

  - Installing sebastian/exporter (1.2.1)
    Loading from cache

  - Installing sebastian/environment (1.3.7)
    Loading from cache

  - Installing sebastian/diff (1.4.1)
    Loading from cache

  - Installing sebastian/comparator (1.2.0)
    Loading from cache

  - Installing doctrine/instantiator (1.0.5)
    Loading from cache

  - Installing webmozart/assert (1.0.2)
    Downloading: 100%         

  - Installing phpdocumentor/reflection-common (1.0)
    Downloading: 100%         

  - Installing phpdocumentor/type-resolver (0.2)
    Downloading: 100%         

  - Installing phpdocumentor/reflection-docblock (3.1.0)
    Downloading: 100%         

  - Installing phpspec/prophecy (v1.6.1)
    Downloading: 100%         

  - Installing phpunit/php-text-template (1.2.1)
    Loading from cache

  - Installing phpunit/phpunit-mock-objects (2.3.8)
    Loading from cache

  - Installing phpunit/php-timer (1.0.8)
    Loading from cache

  - Installing phpunit/php-token-stream (1.4.8)
    Loading from cache

  - Installing phpunit/php-file-iterator (1.3.4)
    Downloading: 100%         

  - Installing phpunit/php-code-coverage (2.2.4)
    Loading from cache

  - Installing phpunit/phpunit (4.5.1)
    Downloading: 100%         

symfony/console suggests installing symfony/process ()
symfony/event-dispatcher suggests installing symfony/dependency-injection ()
symfony/event-dispatcher suggests installing symfony/http-kernel ()
guzzle/guzzle suggests installing guzzlehttp/guzzle (Guzzle 5 has moved to a new package name. The package you have installed, Guzzle 3, is deprecated.)
satooshi/php-coveralls suggests installing symfony/http-kernel (Allows Symfony integration)
sebastian/global-state suggests installing ext-uopz (*)
phpunit/php-code-coverage suggests installing ext-xdebug (>=2.2.1)
phpunit/phpunit suggests installing phpunit/php-invoker (~1.1)
Writing lock file
Generating autoload files
lewis@lewis-HP-ENVY-17-Notebook-PC:~/projects/php/bigcommerce-api-php$ phpunit
PHPUnit 5.3.0 by Sebastian Bergmann and contributors.

Error:         No code coverage driver is available

...............................................................  63 / 199 ( 31%)
............................................................... 126 / 199 ( 63%)
............................................................... 189 / 199 ( 94%)
..........                                                      199 / 199 (100%)

Time: 559 ms, Memory: 10.00Mb

OK (199 tests, 352 assertions)
lewis@lewis-HP-ENVY-17-Notebook-PC:~/projects/php/bigcommerce-api-php$

Need to know if I have actually broken something or if what you mean is this breaks a third-party system somewhere down the line (i.e. it's a deck of cards where some third-party internal tools require this to work)

bc-ruth · 2016-06-13T20:01:23Z

Thanks @Lewiscowles1986

The original issue is described in #139

We tried to fix the issue above by removing a line in Connection.php (#155)
While this fix solved the public's use of the API, it broke our internal use of the API.

So we would like to revert #155 but also need to fix the original issue in #139 another way, a fix that would work for both public and internal use.

Lewiscowles1986 · 2016-06-13T20:10:42Z

I've not tested @bc-ruth code in any live projects but it seems to fulfil all needs; I just downloaded and all tests pass (there did seem to be less tests and assertions)

To be clear I don't think I ever had an issue with #139; I just noticed the issue and was put a bit out of joint by the advice to edit the API files directly (seemed like an easy PR) 😉

👍 for this to be merged. I Understand now

bc-ruth force-pushed the OMNI-1167 branch from 6f164a5 to 40c0972 Compare June 10, 2016 18:28

bc-ruth force-pushed the OMNI-1167 branch from 40c0972 to f773ef5 Compare June 10, 2016 21:13

OMNI-1167 Revert bigcommerce#155

407a278

bc-ruth force-pushed the OMNI-1167 branch from f773ef5 to 407a278 Compare June 12, 2016 04:56

bc-ruth changed the title ~~OMNI-1167 Attempt to fix curl_exec(): CURLOPT_INFILE resource has gon…~~ OMNI-1167 Fix curl_exec(): CURLOPT_INFILE resource has gon… Jun 12, 2016

bc-ruth changed the title ~~OMNI-1167 Fix curl_exec(): CURLOPT_INFILE resource has gon…~~ Revert https://github.com/bigcommerce/bigcommerce-api-php/pull/155 Jun 12, 2016

bc-ruth reviewed Jun 12, 2016
View reviewed changes

aleachjr merged commit 37087f7 into bigcommerce:master Jun 15, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Revert https://github.com/bigcommerce/bigcommerce-api-php/pull/155 #174

Revert https://github.com/bigcommerce/bigcommerce-api-php/pull/155 #174

Uh oh!

bc-ruth commented Jun 10, 2016 •

edited

Loading

Uh oh!

coveralls commented Jun 10, 2016 •

edited

Loading

Uh oh!

coveralls commented Jun 10, 2016 •

edited

Loading

Uh oh!

lord2800 commented Jun 10, 2016

Uh oh!

coveralls commented Jun 10, 2016 •

edited

Loading

Uh oh!

coveralls commented Jun 12, 2016 •

edited

Loading

Uh oh!

PascalZajac commented Jun 12, 2016

Uh oh!

bc-ruth Jun 12, 2016

Uh oh!

bc-ruth commented Jun 12, 2016

Uh oh!

aleachjr commented Jun 12, 2016

Uh oh!

PascalZajac commented Jun 13, 2016

Uh oh!

bc-ruth commented Jun 13, 2016

Uh oh!

Lewiscowles1986 commented Jun 13, 2016 •

edited

Loading

Uh oh!

bc-ruth commented Jun 13, 2016

Uh oh!

Lewiscowles1986 commented Jun 13, 2016 •

edited

Loading

Uh oh!

Uh oh!

Revert https://github.com/bigcommerce/bigcommerce-api-php/pull/155 #174

Revert https://github.com/bigcommerce/bigcommerce-api-php/pull/155 #174

Uh oh!

Conversation

bc-ruth commented Jun 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jun 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jun 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lord2800 commented Jun 10, 2016

Uh oh!

coveralls commented Jun 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jun 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PascalZajac commented Jun 12, 2016

Uh oh!

bc-ruth Jun 12, 2016

Choose a reason for hiding this comment

Uh oh!

bc-ruth commented Jun 12, 2016

Uh oh!

aleachjr commented Jun 12, 2016

Uh oh!

PascalZajac commented Jun 13, 2016

Uh oh!

bc-ruth commented Jun 13, 2016

Uh oh!

Lewiscowles1986 commented Jun 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bc-ruth commented Jun 13, 2016

Uh oh!

Lewiscowles1986 commented Jun 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

bc-ruth commented Jun 10, 2016 •

edited

Loading

coveralls commented Jun 10, 2016 •

edited

Loading

coveralls commented Jun 10, 2016 •

edited

Loading

coveralls commented Jun 10, 2016 •

edited

Loading

coveralls commented Jun 12, 2016 •

edited

Loading

Lewiscowles1986 commented Jun 13, 2016 •

edited

Loading

Lewiscowles1986 commented Jun 13, 2016 •

edited

Loading