Skip to content

Commit 1c344b2

Browse files
Jellyfrogmurrant
andauthored
PHPUnit should fail on all issues (librenms#17802)
* PHPUnit should fail on all issues * Update test.yml * Update test.yml * Update ErrorReporting.php * Update test.yml * rebase and invert check with early return * Update test.yml * don't use array_push with one arg --------- Co-authored-by: Tony Murray <[email protected]>
1 parent 6c71f74 commit 1c344b2

File tree

2 files changed

+27
-26
lines changed

2 files changed

+27
-26
lines changed

LibreNMS/Util/CiHelper.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,10 @@ public function getFlags(): array
179179
*/
180180
public function checkUnit(): int
181181
{
182-
$phpunit_cmd = [$this->checkPhpExec('phpunit'), '--colors=always'];
182+
$phpunit_cmd = [$this->checkPhpExec('phpunit'), '--colors=always', '--fail-on-all-issues'];
183183

184184
if ($this->flags['fail-fast']) {
185-
array_push($phpunit_cmd, '--stop-on-error', '--stop-on-failure');
185+
$phpunit_cmd[] = '--stop-on-defect';
186186
}
187187

188188
if (Debug::isVerbose()) {

app/Exceptions/ErrorReporting.php

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -157,37 +157,38 @@ public function isReportingEnabled(): bool
157157

158158
private function adjustErrorHandlingForAppEnv(string $environment): void
159159
{
160-
// TODO remove testing and then APP_DEBUG requirement when issues are cleaned up
161-
if ($environment == 'production' || $environment == 'testing' || ! config('app.debug')) {
162-
// in production, don't halt execution on non-fatal errors
163-
set_error_handler(function ($severity, $message, $file, $line) {
164-
// If file is from a package, find the first non-vendor frame
165-
if (str_contains($file, '/vendor/')) {
166-
// add vendor file to message
167-
$message .= ' from ' . strstr($file, 'vendor') . ':' . $line;
168-
[$file, $line] = self::findFirstNonVendorFrame();
169-
}
170-
171-
if ((error_reporting() & $severity) !== 0) { // this check primarily allows @ to suppress errors
172-
error_log("\e[31mPHP Error($severity)\e[0m: $message in $file:$line");
173-
}
174-
175-
// For notices and warnings, prevent conversion to exceptions
176-
if (($severity & (E_NOTICE | E_WARNING | E_USER_NOTICE | E_USER_WARNING | E_DEPRECATED)) !== 0) {
177-
return true; // Prevent the standard error handler from running
178-
}
179-
180-
return false; // For other errors, let Laravel handle them
181-
});
182-
} else {
183-
// non-prod, show deprecations
160+
// throw exceptions and deprecations in testing and non-prod when APP_DEBUG is set.
161+
if ($environment == 'testing' || ($environment !== 'production' && config('app.debug'))) {
184162
app()->booted(function () {
185163
config([
186164
'logging.deprecations.channel' => 'deprecations_channel',
187165
'logging.deprecations.trace' => true,
188166
]);
189167
});
168+
169+
return; // do not override error handler below
190170
}
171+
172+
// in production, don't halt execution on non-fatal errors
173+
set_error_handler(function ($severity, $message, $file, $line) {
174+
// If file is from a package, find the first non-vendor frame
175+
if (str_contains($file, '/vendor/')) {
176+
// add vendor file to message
177+
$message .= ' from ' . strstr($file, 'vendor') . ':' . $line;
178+
[$file, $line] = self::findFirstNonVendorFrame();
179+
}
180+
181+
if ((error_reporting() & $severity) !== 0) { // this check primarily allows @ to suppress errors
182+
error_log("\e[31mPHP Error($severity)\e[0m: $message in $file:$line");
183+
}
184+
185+
// For notices and warnings, prevent conversion to exceptions
186+
if (($severity & (E_NOTICE | E_WARNING | E_USER_NOTICE | E_USER_WARNING | E_DEPRECATED)) !== 0) {
187+
return true; // Prevent the standard error handler from running
188+
}
189+
190+
return false; // For other errors, let Laravel handle them
191+
});
191192
}
192193

193194
private static function findFirstNonVendorFrame(): array

0 commit comments

Comments
 (0)