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

CI: Only enable Whoops when it is tested #5554

Merged
merged 1 commit into from
Sep 2, 2023
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
1 change: 1 addition & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd"

bootstrap="tests/bootstrap.php"
convertDeprecationsToExceptions="true"
colors="true"
verbose="true"
stderr="true"
Expand Down
20 changes: 20 additions & 0 deletions src/Cms/AppErrors.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@
*/
trait AppErrors
{
/**
* Allows to disable Whoops globally in CI;
* can be overridden by explicitly setting
* the `whoops` option to `true` or `false`
*
* @internal
*/
public static bool $enableWhoops = true;

/**
* Whoops instance cache
*/
Expand All @@ -45,6 +54,17 @@ protected function handleCliErrors(): void
*/
protected function handleErrors(): void
{
// no matter the environment, exit early if
// Whoops was disabled globally
// (but continue if the option was explicitly
// set to `true` in the config)
if (
static::$enableWhoops === false &&
$this->option('whoops') !== true
) {
return;
}

if ($this->environment()->cli() === true) {
$this->handleCliErrors();
return;
Expand Down
55 changes: 55 additions & 0 deletions tests/Cms/App/AppErrorsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,37 @@
*/
class AppErrorsTest extends TestCase
{
protected App|null $originalApp;

public function setUp(): void
{
parent::setUp();

// Whoops is normally enabled by default, but disabled in CI
// to reduce memory leaks; in this test class we need it!
$this->originalApp = $this->app;
$this->app = $this->originalApp->clone([
'options' => [
'whoops' => true
]
]);
}

public function tearDown(): void
{
$unsetMethod = new ReflectionMethod(App::class, 'unsetWhoopsHandler');
$unsetMethod->setAccessible(true);

$app = App::instance();
$unsetMethod->invoke($app);
$unsetMethod->invoke($this->app);
$unsetMethod->invoke($this->originalApp);

parent::tearDown();
$this->originalApp = null;

// reset to the value set by tests/bootstrap.php
App::$enableWhoops = false;
}

/**
Expand Down Expand Up @@ -166,6 +188,39 @@ public function testHandleErrors3()
$this->assertInstanceOf('Whoops\Handler\CallbackHandler', $handlers[1]);
}

/**
* @covers ::handleErrors
*/
public function testHandleErrorsGlobalSetting()
{
$whoopsMethod = new ReflectionMethod(App::class, 'whoops');
$whoopsMethod->setAccessible(true);

$testMethod = new ReflectionMethod(App::class, 'handleErrors');
$testMethod->setAccessible(true);

$whoopsEnabled = $whoopsMethod->invoke($this->app);
$whoopsDisabled = $whoopsMethod->invoke($this->originalApp);

$testMethod->invoke($this->app);
$handlers = $whoopsEnabled->getHandlers();
$this->assertCount(2, $handlers);

$testMethod->invoke($this->originalApp);
$handlers = $whoopsDisabled->getHandlers();
$this->assertCount(0, $handlers);

App::$enableWhoops = true;

$testMethod->invoke($this->app);
$handlers = $whoopsEnabled->getHandlers();
$this->assertCount(2, $handlers);

$testMethod->invoke($this->originalApp);
$handlers = $whoopsDisabled->getHandlers();
$this->assertCount(2, $handlers);
}

/**
* @covers ::handleHtmlErrors
* @covers ::getAdditionalWhoopsHandler
Expand Down
4 changes: 3 additions & 1 deletion tests/Cms/App/AppTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -727,12 +727,14 @@ public function testOptionsOnReady()
'error' => $kirby->site()->content()->error()->value(),
'slugs' => 'de'
];
}
},
'whoops' => true
]
]);

$this->assertSame([
'ready' => $ready,
'whoops' => true,
'test' => '/dev/null',
'another.test' => 'foo',
'debug' => true,
Expand Down
13 changes: 10 additions & 3 deletions tests/Cms/Helpers/HelperFunctionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use Kirby\Filesystem\Dir;
use Kirby\Toolkit\Collection;
use Kirby\Toolkit\Obj;
use PHPUnit\Framework\Assert;
use PHPUnit\Framework\Error\Deprecated;

class HelperFunctionsTest extends TestCase
{
Expand Down Expand Up @@ -188,10 +190,15 @@ public function testDeprecated()
{
// the deprecation warnings are always triggered in testing mode,
// so we cannot test it with disabled debug mode
$this->expectException('Whoops\Exception\ErrorException');
$this->expectExceptionMessage('The xyz method is deprecated.');

deprecated('The xyz method is deprecated.');
try {
deprecated('The xyz method is deprecated.');
} catch (Deprecated $e) {
$this->assertSame('The xyz method is deprecated.', $e->getMessage());
return;
}

Assert::fail('Expected deprecation warning was not generated');
}

public function testDumpOnCli()
Expand Down
65 changes: 43 additions & 22 deletions tests/Cms/Helpers/HelpersTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

use Kirby\Exception\InvalidArgumentException;
use Kirby\Toolkit\Obj;
use PHPUnit\Framework\Assert;
use PHPUnit\Framework\Error\Deprecated;
use PHPUnit\Framework\Error\Warning;

/**
* @coversDefaultClass Kirby\Cms\Helpers
Expand Down Expand Up @@ -39,33 +42,46 @@ public function testDeprecated()
{
// the deprecation warnings are always triggered in testing mode,
// so we cannot test it with disabled debug mode
$this->expectException('Whoops\Exception\ErrorException');
$this->expectExceptionMessage('The xyz method is deprecated.');

Helpers::deprecated('The xyz method is deprecated.');
try {
Helpers::deprecated('The xyz method is deprecated.');
} catch (Deprecated $e) {
$this->assertSame('The xyz method is deprecated.', $e->getMessage());
return;
}

Assert::fail('Expected deprecation warning was not generated');
}

/**
* @covers ::deprecated
*/
public function testDeprecatedKeyUndefined()
{
$this->expectException('Whoops\Exception\ErrorException');
$this->expectExceptionMessage('The xyz method is deprecated.');
try {
Helpers::deprecated('The xyz method is deprecated.', 'my-key');
} catch (Deprecated $e) {
$this->assertSame('The xyz method is deprecated.', $e->getMessage());
return;
}

Helpers::deprecated('The xyz method is deprecated.', 'my-key');
Assert::fail('Expected deprecation warning was not generated');
}

/**
* @covers ::deprecated
*/
public function testDeprecatedActivated()
{
$this->expectException('Whoops\Exception\ErrorException');
$this->expectExceptionMessage('The xyz method is deprecated.');
try {
Helpers::$deprecations = ['my-key' => true];
Helpers::deprecated('The xyz method is deprecated.', 'my-key');
} catch (Deprecated $e) {
$this->assertSame('The xyz method is deprecated.', $e->getMessage());
return;
}

Helpers::$deprecations = ['my-key' => true];
Helpers::deprecated('The xyz method is deprecated.', 'my-key');
Assert::fail('Expected deprecation warning was not generated');
}

/**
Expand Down Expand Up @@ -195,19 +211,24 @@ public function testHandleErrorsWarningCaughtCallbackValue()
*/
public function testHandleErrorsWarningNotCaught()
{
$this->expectExceptionMessage('Some warning');

Helpers::handleErrors(
fn () => trigger_error('Some warning', E_USER_WARNING),
function (int $errno, string $errstr) {
$this->assertSame(E_USER_WARNING, $errno);
$this->assertSame('Some warning', $errstr);
try {
Helpers::handleErrors(
fn () => trigger_error('Some warning', E_USER_WARNING),
function (int $errno, string $errstr) {
$this->assertSame(E_USER_WARNING, $errno);
$this->assertSame('Some warning', $errstr);

// continue the handler chain
return false;
},
'handled'
);
} catch (Warning $e) {
$this->assertSame('Some warning', $e->getMessage());
return;
}

// continue the handler chain
return false;
},
'handled'
);
Assert::fail('Expected warning was not generated');
}

/**
Expand Down
6 changes: 6 additions & 0 deletions tests/bootstrap.php
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
<?php

use Kirby\Cms\App;

error_reporting(E_ALL);
ini_set('display_errors', 'on');
ini_set('display_startup_errors', 'on');

require_once __DIR__ . '/../vendor/autoload.php';

define('KIRBY_TESTING', true);

// disable Whoops for all tests that don't need it
// to reduce the impact of memory leaks
App::$enableWhoops = false;