Skip to content

Moved TEST_COMMAND to global section #203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 4, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ php:
- 5.6
- 7.0
- 7.1
- 7.2

env:
- TEST_COMMAND="composer test"
global:
- TEST_COMMAND="composer test"

branches:
except:
Expand All @@ -24,7 +26,7 @@ matrix:
include:
- php: 5.5
env: COMPOSER_FLAGS="--prefer-stable --prefer-lowest" COVERAGE=true TEST_COMMAND="composer test-ci"
- php: 7.1
- php: 7.2
env: DEPENDENCIES=dev
- php: hhvm
dist: trusty
Expand All @@ -35,6 +37,7 @@ matrix:
before_install:
- if [ "$SYMFONY_VERSION" != "" ]; then composer require "symfony/symfony:${SYMFONY_VERSION}" --no-update; fi;
- if [ "$DEPENDENCIES" = "dev" ]; then perl -pi -e 's/^}$/,"minimum-stability":"dev"}/' composer.json; fi;
- if [ $COVERAGE != true ]; then phpenv config-rm xdebug.ini; fi;

install:
- travis_retry composer update ${COMPOSER_FLAGS} --prefer-dist --no-interaction
Expand Down
4 changes: 4 additions & 0 deletions DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -470,15 +470,19 @@ private function createAuthenticationPluginNode()
switch ($config['type']) {
case 'basic':
$this->validateAuthenticationType(['username', 'password'], $config, 'basic');

break;
case 'bearer':
$this->validateAuthenticationType(['token'], $config, 'bearer');

break;
case 'service':
$this->validateAuthenticationType(['service'], $config, 'service');

break;
case 'wsse':
$this->validateAuthenticationType(['username', 'password'], $config, 'wsse');

break;
}

Expand Down
3 changes: 2 additions & 1 deletion DependencyInjection/HttplugExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,8 @@ private function configureClient(ContainerBuilder $container, $clientName, array

$plugins = [];
foreach ($arguments['plugins'] as $plugin) {
list($pluginName, $pluginConfig) = each($plugin);
$pluginName = key($plugin);
$pluginConfig = current($plugin);
if ('reference' === $pluginName) {
$plugins[] = $pluginConfig['id'];
} elseif ('authentication' === $pluginName) {
Expand Down
8 changes: 5 additions & 3 deletions Tests/Functional/ProfilingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@
use Http\Message\Formatter\CurlCommandFormatter;
use Http\Message\Formatter\FullHttpMessageFormatter;
use Http\Mock\Client;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\RequestInterface;
use Symfony\Component\Cache\Adapter\ArrayAdapter;
use Symfony\Component\Stopwatch\Stopwatch;

class ProfilingTest extends \PHPUnit_Framework_TestCase
class ProfilingTest extends TestCase
{
/**
* @var Collector
Expand Down Expand Up @@ -63,14 +64,15 @@ public function testProfilingWithCachePlugin()
$this->assertEquals('example.com', $stack->getRequestHost());
}

/**
* @expectedException \Exception
Copy link
Collaborator

Choose a reason for hiding this comment

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

i discussed once with sebastian bergman and he is saying he agrees that the annotation is less clean than calling setExpectedException. the reason is that if the exception is thrown too early, the test would wrongly be green. this is particularly true when we expect such a generic exception

Copy link
Contributor

Choose a reason for hiding this comment

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

So, it's better to have a if/else at the right position in the test?

Copy link
Collaborator

Choose a reason for hiding this comment

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

why if/else? afaik the $this->setExpectedException call is the best practice way. this makes sure the exception is only considered correct when thrown after that line in the code, and you have namespace resolution.

Copy link
Contributor

Choose a reason for hiding this comment

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

setExpectedException was removed in 6.0 and replaced by expectException. That's why I moved to annotation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, We need the annotation for BC purpose...

Copy link
Contributor

Choose a reason for hiding this comment

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

We can keep this as-is and switch to expectException in next-major.

*/
public function testProfilingWhenPluginThrowException()
{
$client = $this->createClient([
new ExceptionThrowerPlugin(),
]);

$this->setExpectedException(\Exception::class);

try {
$client->sendRequest(new Request('GET', 'https://example.com'));
} finally {
Expand Down
3 changes: 2 additions & 1 deletion Tests/Unit/ClientFactory/BuzzFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
use Http\Adapter\Buzz\Client;
use Http\HttplugBundle\ClientFactory\BuzzFactory;
use Http\Message\MessageFactory;
use PHPUnit\Framework\TestCase;

/**
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
*/
class BuzzFactoryTest extends \PHPUnit_Framework_TestCase
class BuzzFactoryTest extends TestCase
{
public function testCreateClient()
{
Expand Down
3 changes: 2 additions & 1 deletion Tests/Unit/ClientFactory/CurlFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
use Http\Client\Curl\Client;
use Http\Message\MessageFactory;
use Http\Message\StreamFactory;
use PHPUnit\Framework\TestCase;

/**
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
*/
class CurlFactoryTest extends \PHPUnit_Framework_TestCase
class CurlFactoryTest extends TestCase
{
public function testCreateClient()
{
Expand Down
3 changes: 2 additions & 1 deletion Tests/Unit/ClientFactory/Guzzle6FactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@

use Http\HttplugBundle\ClientFactory\Guzzle6Factory;
use Http\Adapter\Guzzle6\Client;
use PHPUnit\Framework\TestCase;

/**
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
*/
class Guzzle6FactoryTest extends \PHPUnit_Framework_TestCase
class Guzzle6FactoryTest extends TestCase
{
public function testCreateClient()
{
Expand Down
3 changes: 2 additions & 1 deletion Tests/Unit/ClientFactory/ReactFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
use Http\Adapter\React\Client;
use Http\HttplugBundle\ClientFactory\ReactFactory;
use Http\Message\MessageFactory;
use PHPUnit\Framework\TestCase;

/**
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
*/
class ReactFactoryTest extends \PHPUnit_Framework_TestCase
class ReactFactoryTest extends TestCase
{
public function testCreateClient()
{
Expand Down
3 changes: 2 additions & 1 deletion Tests/Unit/ClientFactory/SocketFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
use Http\Client\Socket\Client;
use Http\HttplugBundle\ClientFactory\SocketFactory;
use Http\Message\MessageFactory;
use PHPUnit\Framework\TestCase;

/**
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
*/
class SocketFactoryTest extends \PHPUnit_Framework_TestCase
class SocketFactoryTest extends TestCase
{
public function testCreateClient()
{
Expand Down
3 changes: 2 additions & 1 deletion Tests/Unit/Collector/CollectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@

use Http\HttplugBundle\Collector\Collector;
use Http\HttplugBundle\Collector\Stack;
use PHPUnit\Framework\TestCase;

class CollectorTest extends \PHPUnit_Framework_TestCase
class CollectorTest extends TestCase
{
public function testCollectClientNames()
{
Expand Down
3 changes: 2 additions & 1 deletion Tests/Unit/Collector/FormatterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
use Http\HttplugBundle\Collector\Formatter;
use Http\Message\Formatter as MessageFormatter;
use Http\Message\Formatter\CurlCommandFormatter;
use PHPUnit\Framework\TestCase;

class FormatterTest extends \PHPUnit_Framework_TestCase
class FormatterTest extends TestCase
{
/**
* @var MessageFormatter
Expand Down
3 changes: 2 additions & 1 deletion Tests/Unit/Collector/ProfileClientFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
use Http\HttplugBundle\Collector\Formatter;
use Http\HttplugBundle\Collector\ProfileClient;
use Http\HttplugBundle\Collector\ProfileClientFactory;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Stopwatch\Stopwatch;

class ProfileClientFactoryTest extends \PHPUnit_Framework_TestCase
class ProfileClientFactoryTest extends TestCase
{
/**
* @var Collector
Expand Down
3 changes: 2 additions & 1 deletion Tests/Unit/Collector/ProfileClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
use Http\Promise\FulfilledPromise;
use Http\Promise\Promise;
use Http\Promise\RejectedPromise;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\UriInterface;
use Symfony\Component\Stopwatch\Stopwatch;
use Symfony\Component\Stopwatch\StopwatchEvent;

class ProfileClientTest extends \PHPUnit_Framework_TestCase
class ProfileClientTest extends TestCase
{
/**
* @var Collector
Expand Down
8 changes: 5 additions & 3 deletions Tests/Unit/Collector/ProfilePluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
use Http\Promise\FulfilledPromise;
use Http\Promise\Promise;
use Http\Promise\RejectedPromise;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;

class ProfilePluginTest extends \PHPUnit_Framework_TestCase
class ProfilePluginTest extends TestCase
{
/**
* @var Plugin
Expand Down Expand Up @@ -167,10 +168,11 @@ public function testOnFulfilled()
$this->assertEquals('FormattedResponse', $profile->getResponse());
}

/**
* @expectedException \Http\Client\Exception\TransferException
*/
public function testOnRejected()
{
$this->setExpectedException(TransferException::class);

$promise = $this->subject->handleRequest($this->request, function () {
return $this->rejectedPromise;
}, function () {
Expand Down
25 changes: 14 additions & 11 deletions Tests/Unit/Collector/StackPluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Http\HttplugBundle\Tests\Unit\Collector;

use Exception;
use GuzzleHttp\Psr7\Request;
use GuzzleHttp\Psr7\Response;
use Http\Client\Exception\HttpException;
Expand All @@ -12,10 +11,12 @@
use Http\HttplugBundle\Collector\StackPlugin;
use Http\Promise\FulfilledPromise;
use Http\Promise\RejectedPromise;
use PHPUnit\Framework\Error\Warning;
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;

class StackPluginTest extends \PHPUnit_Framework_TestCase
class StackPluginTest extends TestCase
{
/**
* @var Collector
Expand All @@ -38,7 +39,7 @@ class StackPluginTest extends \PHPUnit_Framework_TestCase
private $response;

/**
* @var Exception
* @var \Exception
*/
private $exception;

Expand Down Expand Up @@ -130,6 +131,9 @@ public function testOnFulfilled()
$this->assertEquals('FormattedResponse', $currentStack->getResponse());
}

/**
* @expectedException \Exception
*/
public function testOnRejected()
{
//Capture the current stack
Expand All @@ -147,8 +151,6 @@ public function testOnRejected()
->method('deactivateStack')
;

$this->setExpectedException(Exception::class);

$next = function () {
return new RejectedPromise($this->exception);
};
Expand All @@ -162,15 +164,16 @@ public function testOnRejected()
$this->assertTrue($currentStack->isFailed());
}

/**
* @expectedException \Exception
*/
public function testOnException()
{
$this->collector
->expects($this->once())
->method('deactivateStack')
;

$this->setExpectedException(\Exception::class);

$next = function () {
throw new \Exception();
};
Expand All @@ -181,18 +184,18 @@ public function testOnException()

public function testOnError()
{
if (!interface_exists(\Throwable::class)) {
if (PHP_VERSION_ID <= 70000) {
$this->markTestSkipped();
}

//PHPUnit wrap any \Error into a \PHPUnit\Framework\Error\Warning.
$this->expectException(Warning::class);

$this->collector
->expects($this->once())
->method('deactivateStack')
;

//PHPUnit wrap any \Error into a \PHPUnit_Framework_Error. So we are expecting the
$this->setExpectedException(\PHPUnit_Framework_Error::class);

$next = function () {
return 2 / 0;
};
Expand Down
3 changes: 2 additions & 1 deletion Tests/Unit/Discovery/ConfiguredClientsStrategyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
use Http\Client\HttpAsyncClient;
use Http\Client\HttpClient;
use Http\HttplugBundle\Discovery\ConfiguredClientsStrategy;
use PHPUnit\Framework\TestCase;

class ConfiguredClientsStrategyTest extends \PHPUnit_Framework_TestCase
class ConfiguredClientsStrategyTest extends TestCase
{
public function testGetCandidates()
{
Expand Down
12 changes: 9 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@
"php-http/message": "^1.4",
"php-http/discovery": "^1.0",
"twig/twig": "^1.18 || ^2.0",
"symfony/asset": "^2.8 || ^3.0"
"symfony/asset": "^2.8 || ^3.0",
"symfony/symfony": "2.8.27"
},
"require-dev": {
"phpunit/phpunit": "^4.5 || ^5.4",
"phpunit/phpunit": "^4.8.35 || ^5.7 || ^6.1",
"phpunit/php-token-stream": "^1.1.8",
"php-http/curl-client": "^1.0",
"php-http/socket-client": "^1.0",
Expand All @@ -49,7 +50,7 @@
"symfony/browser-kit": "^2.8 || ^3.0",
"symfony/dom-crawler": "^2.8 || ^3.0",
"polishsymfonycommunity/symfony-mocker-container": "^1.0",
"matthiasnoback/symfony-dependency-injection-test": "^1.0",
"matthiasnoback/symfony-dependency-injection-test": "^1.0 || ^2.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need both versions to support the range of php versions we support?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we can't install PHPUnit 6.0 because of a conflict on sebastian/exporter.

"nyholm/nsa": "^1.1"
},
"conflict": {
Expand All @@ -60,6 +61,11 @@
"Http\\HttplugBundle\\": ""
}
},
"autoload-dev": {
"classmap": [
"Tests/Resources/app/AppKernel.php"
]
},
"scripts": {
"test": "vendor/bin/phpunit",
"test-ci": "vendor/bin/phpunit --coverage-text --coverage-clover=build/coverage.xml"
Expand Down
1 change: 1 addition & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@

<php>
<server name="KERNEL_DIR" value="./Tests/Resources/app" />
<server name="KERNEL_CLASS" value="AppKernel" />
</php>
</phpunit>