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

Prevent file IO when not strictly necessary #5742

Merged
merged 1 commit into from
Mar 12, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions .psalm/baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -927,8 +927,6 @@
<code><![CDATA[listTestsXml]]></code>
<code><![CDATA[logEventsText]]></code>
<code><![CDATA[logEventsText]]></code>
<code><![CDATA[logEventsText]]></code>
<code><![CDATA[logEventsVerboseText]]></code>
<code><![CDATA[logEventsVerboseText]]></code>
<code><![CDATA[logEventsVerboseText]]></code>
<code><![CDATA[logfileJunit]]></code>
Expand Down
2 changes: 1 addition & 1 deletion src/Framework/TestSuite.php
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public function addTestSuite(ReflectionClass $testClass): void
*/
public function addTestFile(string $filename): void
{
if (is_file($filename) && str_ends_with($filename, '.phpt')) {
if (str_ends_with($filename, '.phpt') && is_file($filename)) {
try {
$this->addTest(new PhptTestCase($filename));
} catch (RunnerException $e) {
Expand Down
7 changes: 4 additions & 3 deletions src/Runner/Baseline/Issue.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,13 @@ public function equals(self $other): bool
*/
private static function calculateHash(string $file, int $line): string
{
if (!is_file($file)) {
$lines = @file($file, FILE_IGNORE_NEW_LINES);

if ($lines === false && !is_file($file)) {
throw new FileDoesNotExistException($file);
}

$lines = file($file, FILE_IGNORE_NEW_LINES);
$key = $line - 1;
$key = $line - 1;

if (!isset($lines[$key])) {
throw new FileDoesNotHaveLineException($file, $line);
Expand Down
12 changes: 5 additions & 7 deletions src/Runner/PhptTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -638,15 +638,13 @@
$coverage = RawCodeCoverageData::fromXdebugWithoutPathCoverage([]);
$files = $this->getCoverageFiles();

if (is_file($files['coverage'])) {
$buffer = @file_get_contents($files['coverage']);
$buffer = @file_get_contents($files['coverage']);

Check warning on line 641 in src/Runner/PhptTestCase.php

View check run for this annotation

Codecov / codecov/patch

src/Runner/PhptTestCase.php#L641

Added line #L641 was not covered by tests

if ($buffer !== false) {
$coverage = @unserialize($buffer);
if ($buffer !== false) {
$coverage = @unserialize($buffer);

Check warning on line 644 in src/Runner/PhptTestCase.php

View check run for this annotation

Codecov / codecov/patch

src/Runner/PhptTestCase.php#L643-L644

Added lines #L643 - L644 were not covered by tests

if ($coverage === false) {
$coverage = RawCodeCoverageData::fromXdebugWithoutPathCoverage([]);
}
if ($coverage === false) {
$coverage = RawCodeCoverageData::fromXdebugWithoutPathCoverage([]);

Check warning on line 647 in src/Runner/PhptTestCase.php

View check run for this annotation

Codecov / codecov/patch

src/Runner/PhptTestCase.php#L646-L647

Added lines #L646 - L647 were not covered by tests
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/Runner/ResultCache/DefaultResultCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use function file_put_contents;
use function is_array;
use function is_dir;
use function is_file;
use function json_decode;
use function json_encode;
use PHPUnit\Framework\TestStatus\TestStatus;
Expand Down Expand Up @@ -86,12 +85,14 @@ public function time(string $id): float

public function load(): void
{
if (!is_file($this->cacheFilename)) {
$contents = @file_get_contents($this->cacheFilename);

if ($contents === false) {
return;
}

$data = json_decode(
file_get_contents($this->cacheFilename),
$contents,
true,
);

Expand Down
9 changes: 2 additions & 7 deletions src/TextUI/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
namespace PHPUnit\TextUI;

use const PHP_EOL;
use function is_file;
use function is_readable;
use function printf;
use function realpath;
Expand Down Expand Up @@ -517,9 +516,7 @@ private function writeRandomSeedInformation(Printer $printer, Configuration $con
private function registerLogfileWriters(Configuration $configuration): void
{
if ($configuration->hasLogEventsText()) {
if (is_file($configuration->logEventsText())) {
unlink($configuration->logEventsText());
}
@unlink($configuration->logEventsText());

EventFacade::instance()->registerTracer(
new EventLogger(
Expand All @@ -530,9 +527,7 @@ private function registerLogfileWriters(Configuration $configuration): void
}

if ($configuration->hasLogEventsVerboseText()) {
if (is_file($configuration->logEventsVerboseText())) {
unlink($configuration->logEventsVerboseText());
}
@unlink($configuration->logEventsVerboseText());

EventFacade::instance()->registerTracer(
new EventLogger(
Expand Down
2 changes: 1 addition & 1 deletion src/TextUI/Configuration/Cli/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ public function fromParameters(array $parameters): Configuration
case '--use-baseline':
$useBaseline = $option[1];

if (!is_file($useBaseline) && basename($useBaseline) === $useBaseline) {
if (basename($useBaseline) === $useBaseline && !is_file($useBaseline)) {
$useBaseline = getcwd() . DIRECTORY_SEPARATOR . $useBaseline;
}

Expand Down
14 changes: 7 additions & 7 deletions src/TextUI/Configuration/TestSuiteBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,13 @@ public function build(Configuration $configuration): TestSuite
*/
private function testSuiteFromPath(string $path, array $suffixes, ?TestSuite $suite = null): TestSuite
{
if (str_ends_with($path, '.phpt') && is_file($path)) {
$suite = $suite ?: TestSuite::empty($path);
$suite->addTestFile($path);

return $suite;
}

if (is_dir($path)) {
$files = (new FileIteratorFacade)->getFilesAsArray($path, $suffixes);

Expand All @@ -100,13 +107,6 @@ private function testSuiteFromPath(string $path, array $suffixes, ?TestSuite $su
return $suite;
}

if (is_file($path) && str_ends_with($path, '.phpt')) {
$suite = $suite ?: TestSuite::empty($path);
$suite->addTestFile($path);

return $suite;
}

try {
$testClass = (new TestSuiteLoader)->load($path);
} catch (Exception $e) {
Expand Down
6 changes: 3 additions & 3 deletions src/Util/Filter.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ private static function shouldPrintFrame(array $frame, false|string $prefix, Exc
$script = '';
}

return is_file($file) &&
return $fileIsNotPrefixed &&
$file !== $script &&
self::fileIsExcluded($file, $excludeList) &&
$fileIsNotPrefixed &&
$file !== $script;
is_file($file);
}

private static function fileIsExcluded(string $file, ExcludeList $excludeList): bool
Expand Down
8 changes: 4 additions & 4 deletions src/Util/PHP/AbstractPhpProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use function array_merge;
use function assert;
use function escapeshellarg;
use function file_exists;
use function file_get_contents;
use function ini_get_all;
use function restore_error_handler;
Expand Down Expand Up @@ -139,12 +138,13 @@
{
$_result = $this->runJob($job);

$processResult = '';
$processResult = @file_get_contents($processResultFile);

if (file_exists($processResultFile)) {
$processResult = file_get_contents($processResultFile);
if ($processResult !== false) {

@unlink($processResultFile);
} else {
$processResult = '';

Check warning on line 147 in src/Util/PHP/AbstractPhpProcess.php

View check run for this annotation

Codecov / codecov/patch

src/Util/PHP/AbstractPhpProcess.php#L147

Added line #L147 was not covered by tests
}

$this->processChildResult(
Expand Down