Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

Added Lorempixel check for ImageTest.php #866

Merged
merged 4 commits into from
Mar 29, 2016
Merged

Added Lorempixel check for ImageTest.php #866

merged 4 commits into from
Mar 29, 2016

Conversation

jremes-foss
Copy link
Contributor

Added curl check to file test/Faker/Provider/ImageTest.php to test LoremPixel before conducting the unit tests.

Basically, this check utilises curl to retrieve HTTP header. If header includes response code which is between 200 and 299, unit tests are being conducted. Otherwise, they are skipped.

$httpCode = curl_getinfo($curlPing, CURLINFO_HTTP_CODE);
curl_close($curlPing);

if ($httpCode >= 200 && $httpCode < 300)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest you reverse the logic:

if ($httpCode < 200 || $httpCode > 300) {
   $this-> markTestSkipped('LoremPixel is offline, skipping image download');
  return;
}

cf. http://refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, this can be done.

@jremes-foss
Copy link
Contributor Author

Looks like the ImageTest check works as intended:

1) Faker\Test\Provider\ImageTest::testDownloadWithDefaults
LoremPixel is offline, skipping image download
/home/travis/build/fzaninotto/Faker/test/Faker/Provider/ImageTest.php:59

curl_close($curlPing);

/**
* If HTTP response is something else than 200-299, skip the test
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary, the code speaks for itself

@fzaninotto fzaninotto merged commit a9e41bc into fzaninotto:master Mar 29, 2016
@fzaninotto
Copy link
Owner

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants