Skip to content
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

Add --default-time-limit #3224

Merged
merged 12 commits into from Sep 8, 2018
Merged
1 change: 1 addition & 0 deletions phpunit.xsd
Expand Up @@ -258,6 +258,7 @@
<xs:attribute name="beStrictAboutTestsThatDoNotTestAnything" type="xs:boolean" default="true"/>
<xs:attribute name="beStrictAboutTodoAnnotatedTests" type="xs:boolean" default="false"/>
<xs:attribute name="beStrictAboutCoversAnnotation" type="xs:boolean" default="false"/>
<xs:attribute name="defaultTimeLimit" type="xs:integer" default="0"/>
<xs:attribute name="enforceTimeLimit" type="xs:boolean" default="false"/>
<xs:attribute name="ignoreDeprecatedCodeUnitsFromCodeCoverage" type="xs:boolean" default="false"/>
<xs:attribute name="timeoutForSmallTests" type="xs:integer" default="1"/>
Expand Down
20 changes: 19 additions & 1 deletion src/Framework/TestResult.php
Expand Up @@ -139,6 +139,11 @@ class TestResult implements Countable
*/
protected $beStrictAboutResourceUsageDuringSmallTests = false;

/**
* @var int
*/
private $defaultTimeLimit = 0;

/**
* @var bool
*/
Expand Down Expand Up @@ -639,8 +644,8 @@ public function run(Test $test): void

try {
if (!$test instanceof WarningTestCase &&
$test->getSize() != \PHPUnit\Util\Test::UNKNOWN &&
$this->enforceTimeLimit &&
($this->defaultTimeLimit || $test->getSize() != \PHPUnit\Util\Test::UNKNOWN) &&
\extension_loaded('pcntl') && \class_exists(Invoker::class)) {
switch ($test->getSize()) {
case \PHPUnit\Util\Test::SMALL:
Expand All @@ -656,6 +661,11 @@ public function run(Test $test): void
case \PHPUnit\Util\Test::LARGE:
$_timeout = $this->timeoutForLargeTests;

break;

case \PHPUnit\Util\Test::UNKNOWN:
$_timeout = $this->defaultTimeLimit;

break;
}

Expand Down Expand Up @@ -1061,6 +1071,14 @@ public function wasSuccessful(): bool
return empty($this->errors) && empty($this->failures) && empty($this->warnings);
}

/**
* Sets the default timeout for tests
*/
public function setDefaultTimeLimit(int $timeout): void
{
$this->defaultTimeLimit = $timeout;
}

/**
* Sets the timeout for small tests.
*/
Expand Down
7 changes: 7 additions & 0 deletions src/TextUI/Command.php
Expand Up @@ -84,6 +84,7 @@ class Command
'disallow-test-output' => null,
'disallow-resource-usage' => null,
'disallow-todo-tests' => null,
'default-time-limit=' => null,
Copy link
Owner

Choose a reason for hiding this comment

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

For consistency, I would rather not add an option like --default-time-limit to set the default time limit on the CLI. The timeouts for @small, @medium, and @large can also only be configured in the configuration file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is true. I asked @reinholdfuereder what the exact use case would be and this time limit is meant to be for tests that are not marked with a @size: #2085 (comment)

I know a bunch of older test collections with legacy would benefit from this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure about the trailing=, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The = is required for --key=value, right? I have not found a clean way of doing --key[=value].

Copy link
Owner

Choose a reason for hiding this comment

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

The = means that the option requires a value.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for clarifying, @sebastianbergmann!

'enforce-time-limit' => null,
'exclude-group=' => null,
'filter=' => null,
Expand Down Expand Up @@ -692,6 +693,11 @@ protected function handleArguments(array $argv): void

break;

case '--default-time-limit':
$this->arguments['defaultTimeLimit'] = (int) $option[1];

break;

case '--enforce-time-limit':
$this->arguments['enforceTimeLimit'] = true;

Expand Down Expand Up @@ -1120,6 +1126,7 @@ protected function showHelp(): void
--disallow-test-output Be strict about output during tests
--disallow-resource-usage Be strict about resource usage during small tests
--enforce-time-limit Enforce time limit based on test size
--default-time-limit=<sec> Timeout in seconds for tests without @small, @medium or @large
--disallow-todo-tests Disallow @todo-annotated tests

--process-isolation Run each test in a separate PHP process
Expand Down
17 changes: 17 additions & 0 deletions src/TextUI/TestRunner.php
Expand Up @@ -54,6 +54,7 @@
use SebastianBergmann\CodeCoverage\Report\Xml\Facade as XmlReport;
use SebastianBergmann\Comparator\Comparator;
use SebastianBergmann\Environment\Runtime;
use SebastianBergmann\Invoker\Invoker;

/**
* A TestRunner for the Command Line Interface (CLI)
Expand Down Expand Up @@ -579,7 +580,18 @@ public function doRun(Test $suite, array $arguments = [], bool $exit = true): Te
$result->beStrictAboutOutputDuringTests($arguments['disallowTestOutput']);
$result->beStrictAboutTodoAnnotatedTests($arguments['disallowTodoAnnotatedTests']);
$result->beStrictAboutResourceUsageDuringSmallTests($arguments['beStrictAboutResourceUsageDuringSmallTests']);

if ($arguments['enforceTimeLimit'] === true) {
if (!\class_exists(Invoker::class)) {
$this->writeMessage('Error', 'Package phpunit/php-invoker is required for enforcing time limits');
}

if (!\extension_loaded('pcntl') || \strpos(\ini_get('disable_functions'), 'pcntl') !== false) {
$this->writeMessage('Error', 'PHP extension pcntl is required for enforcing time limits');
}
}
$result->enforceTimeLimit($arguments['enforceTimeLimit']);
$result->setDefaultTimeLimit($arguments['defaultTimeLimit']);
$result->setTimeoutForSmallTests($arguments['timeoutForSmallTests']);
$result->setTimeoutForMediumTests($arguments['timeoutForMediumTests']);
$result->setTimeoutForLargeTests($arguments['timeoutForLargeTests']);
Expand Down Expand Up @@ -942,6 +954,10 @@ protected function handleConfiguration(array &$arguments): void
$arguments['disallowTestOutput'] = $phpunitConfiguration['disallowTestOutput'];
}

if (isset($phpunitConfiguration['defaultTimeLimit']) && !isset($arguments['defaultTimeLimit'])) {
$arguments['defaultTimeLimit'] = $phpunitConfiguration['defaultTimeLimit'];
}

if (isset($phpunitConfiguration['enforceTimeLimit']) && !isset($arguments['enforceTimeLimit'])) {
$arguments['enforceTimeLimit'] = $phpunitConfiguration['enforceTimeLimit'];
}
Expand Down Expand Up @@ -1179,6 +1195,7 @@ protected function handleConfiguration(array &$arguments): void
$arguments['crap4jThreshold'] = $arguments['crap4jThreshold'] ?? 30;
$arguments['disallowTestOutput'] = $arguments['disallowTestOutput'] ?? false;
$arguments['disallowTodoAnnotatedTests'] = $arguments['disallowTodoAnnotatedTests'] ?? false;
$arguments['defaultTimeLimit'] = $arguments['defaultTimeLimit'] ?? 0;
$arguments['enforceTimeLimit'] = $arguments['enforceTimeLimit'] ?? false;
$arguments['excludeGroups'] = $arguments['excludeGroups'] ?? [];
$arguments['failOnRisky'] = $arguments['failOnRisky'] ?? false;
Expand Down
8 changes: 8 additions & 0 deletions src/Util/Configuration.php
Expand Up @@ -59,6 +59,7 @@
* beStrictAboutResourceUsageDuringSmallTests="false"
* beStrictAboutTestsThatDoNotTestAnything="false"
* beStrictAboutTodoAnnotatedTests="false"
* defaultTimeLimit="0"
* enforceTimeLimit="false"
* ignoreDeprecatedCodeUnitsFromCodeCoverage="false"
* timeoutForSmallTests="1"
Expand Down Expand Up @@ -855,6 +856,13 @@ public function getPHPUnitConfiguration(): array
);
}

if ($root->hasAttribute('defaultTimeLimit')) {
$result['defaultTimeLimit'] = $this->getInteger(
(string) $root->getAttribute('defaultTimeLimit'),
1
);
}

if ($root->hasAttribute('enforceTimeLimit')) {
$result['enforceTimeLimit'] = $this->getBoolean(
(string) $root->getAttribute('enforceTimeLimit'),
Expand Down
20 changes: 20 additions & 0 deletions tests/Regression/GitHub/2085/Issue2085Test.php
@@ -0,0 +1,20 @@
<?php
/*
* This file is part of PHPUnit.
*
* (c) Sebastian Bergmann <sebastian@phpunit.de>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
use PHPUnit\Framework\TestCase;

class Issue2085Test extends TestCase
{
public function testShouldAbortSlowTestByEnforcingTimeLimit(): void
{
$this->assertTrue(true);
\sleep(1.2);
$this->assertTrue(true);
}
}
@@ -0,0 +1,2 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit enforceTimeLimit="true" defaultTimeLimit="1" />
@@ -0,0 +1,31 @@
--TEST--
Test XML config enforceTimeLimit, defaultTimeLimit without php-invoker, with pcntl
--SKIPIF--
<?php
if (\class_exists(Invoker::class)) {
print "Skip: package phpunit/php-invoker is installed" . PHP_EOL;
}

if (!\extension_loaded('pcntl') || \strpos(\ini_get('disable_functions'), 'pcntl') !== false) {
print "Skip: extension pcntl is required for enforcing time limits" . PHP_EOL;
}
--DESCRIPTION--
https://github.com/sebastianbergmann/phpunit/issues/2085
--FILE--
<?php
$_SERVER['argv'][1] = '-c';
$_SERVER['argv'][2] = __DIR__ . '/configuration_enforce_time_limit_options.xml';
$_SERVER['argv'][3] = __DIR__ . '/Issue2085Test.php';

require __DIR__ . '/../../../bootstrap.php';
PHPUnit\TextUI\Command::main();
--EXPECTF--
PHPUnit %s by Sebastian Bergmann and contributors.


Error: Package phpunit/php-invoker is required for enforcing time limits
. 1 / 1 (100%)

Time: %s, Memory: %s

OK (1 test, 2 assertions)
32 changes: 32 additions & 0 deletions tests/Regression/GitHub/2085/issue-2085-test-without-invoker.phpt
@@ -0,0 +1,32 @@
--TEST--
Test CLI flags --enforce-time-limit --default-time-limit without php-invoker, with pcntl
--DESCRIPTION--
https://github.com/sebastianbergmann/phpunit/issues/2085
--SKIPIF--
<?php
if (\class_exists(Invoker::class)) {
print "Skip: package phpunit/php-invoker is installed" . PHP_EOL;
}

if (!\extension_loaded('pcntl') || \strpos(\ini_get('disable_functions'), 'pcntl') !== false) {
print "Skip: extension pcntl is required for enforcing time limits" . PHP_EOL;
}
--FILE--
<?php
$_SERVER['argv'][1] = '--no-configuration';
$_SERVER['argv'][2] = '--enforce-time-limit';
$_SERVER['argv'][3] = '--default-time-limit=10';
$_SERVER['argv'][4] = __DIR__ . '/Issue2085Test.php';

require __DIR__ . '/../../../bootstrap.php';
PHPUnit\TextUI\Command::main();
--EXPECTF--
PHPUnit %s by Sebastian Bergmann and contributors.


Error: Package phpunit/php-invoker is required for enforcing time limits
. 1 / 1 (100%)

Time: %s, Memory: %s

OK (1 test, 2 assertions)
36 changes: 36 additions & 0 deletions tests/Regression/GitHub/2085/issue-2085-test.phpt
@@ -0,0 +1,36 @@
--TEST--
Test CLI flags --enforce-time-limit --default-time-limit
--DESCRIPTION--
https://github.com/sebastianbergmann/phpunit/issues/2085
--SKIPIF--
<?php
if (!\class_exists(Invoker::class)) {
print "Skip: package phpunit/php-invoker is required for enforcing time limits" . PHP_EOL;
}

if (!\extension_loaded('pcntl') || \strpos(\ini_get('disable_functions'), 'pcntl') !== false) {
print "Skip: extension pcntl is required for enforcing time limits" . PHP_EOL;
}
--FILE--
<?php
$_SERVER['argv'][1] = '--no-configuration';
$_SERVER['argv'][2] = '--enforce-time-limit';
$_SERVER['argv'][3] = '--default-time-limit=1';
$_SERVER['argv'][4] = __DIR__ . '/Issue2085Test.php';

require __DIR__ . '/../../../bootstrap.php';
PHPUnit\TextUI\Command::main();
--EXPECTF--
PHPUnit %s by Sebastian Bergmann and contributors.

R 1 / 1 (100%)

Time: %s, Memory: %s

There was 1 risky test:

1) Issue2085Test::testShouldAbortSlowTestByEnforcingTimeLimit
Execution aborted after 1 second

OK, but incomplete, skipped, or risky tests!
Tests: 1, Assertions: 1, Risky: 1.
1 change: 1 addition & 0 deletions tests/_files/configuration.xml
Expand Up @@ -25,6 +25,7 @@
beStrictAboutTestsThatDoNotTestAnything="false"
beStrictAboutTodoAnnotatedTests="false"
beStrictAboutCoversAnnotation="false"
defaultTimeLimit="123"
enforceTimeLimit="false"
ignoreDeprecatedCodeUnitsFromCodeCoverage="false"
timeoutForSmallTests="1"
Expand Down
1 change: 1 addition & 0 deletions tests/_files/configuration_xinclude.xml
Expand Up @@ -26,6 +26,7 @@
beStrictAboutTestsThatDoNotTestAnything="false"
beStrictAboutTodoAnnotatedTests="false"
beStrictAboutCoversAnnotation="false"
defaultTimeLimit="123"
enforceTimeLimit="false"
ignoreDeprecatedCodeUnitsFromCodeCoverage="false"
timeoutForSmallTests="1"
Expand Down
1 change: 1 addition & 0 deletions tests/end-to-end/help.phpt
Expand Up @@ -56,6 +56,7 @@ Test Execution Options:
--disallow-test-output Be strict about output during tests
--disallow-resource-usage Be strict about resource usage during small tests
--enforce-time-limit Enforce time limit based on test size
--default-time-limit=<sec> Timeout in seconds for tests without @small, @medium or @large
--disallow-todo-tests Disallow @todo-annotated tests

--process-isolation Run each test in a separate PHP process
Expand Down
1 change: 1 addition & 0 deletions tests/end-to-end/help2.phpt
Expand Up @@ -57,6 +57,7 @@ Test Execution Options:
--disallow-test-output Be strict about output during tests
--disallow-resource-usage Be strict about resource usage during small tests
--enforce-time-limit Enforce time limit based on test size
--default-time-limit=<sec> Timeout in seconds for tests without @small, @medium or @large
--disallow-todo-tests Disallow @todo-annotated tests

--process-isolation Run each test in a separate PHP process
Expand Down
1 change: 1 addition & 0 deletions tests/unit/Util/ConfigurationTest.php
Expand Up @@ -471,6 +471,7 @@ public function testPHPUnitConfigurationIsReadCorrectly(): void
'reportUselessTests' => false,
'strictCoverage' => false,
'disallowTestOutput' => false,
'defaultTimeLimit' => 123,
'enforceTimeLimit' => false,
'extensionsDirectory' => '/tmp',
'printerClass' => 'PHPUnit\TextUI\ResultPrinter',
Expand Down