Skip to content

fix: don't warn from Trace::cleanup() when there is no traces directory to remove - #278

Open
joshdaugherty wants to merge 1 commit into
pestphp:5.xfrom
joshdaugherty:fix-trace-cleanup-warning
Open

joshdaugherty wants to merge 1 commit into
pestphp:5.xfrom
joshdaugherty:fix-trace-cleanup-warning

Conversation

@joshdaugherty

Copy link
Copy Markdown

Fixes pestphp/pest#1944.

What happens

Trace::cleanup(), added in v5.1.0, runs once per run whether or not --trace was passed, and ends with @rmdir(self::dir()). When tests/Browser/Traces does not exist, rmdir() raises a warning, and it does the same for a directory that holds anything other than *.zip. The @ hides the message, but PHPUnit still records it at test-runner level, as Test Runner Triggered PHP Warning (suppressed using operator). A suite with failOnWarning="true" then exits 1 with every test passing, and nothing in the output says why.

The change

cleanup() removes the directory only when it exists and is empty, so rmdir() has nothing to warn about and the @ is no longer needed. The *.zip removal above it is unchanged.

Checks

Against a Laravel package's browser suite of 254 passing tests with failOnWarning="true" (macOS, PHP 8.5.8, Pest 5.2.1, PHPUnit 13.3.4), with this change applied to the installed v5.1.0:

Trace.php tests/Browser/Traces before the run Exit Warnings recorded
v5.1.0 absent 1 1
this change absent 0 0
this change present, holding a non-zip file 0 0 (directory kept)

In this repository: Pint, Rector, PHPStan and type coverage pass. The full suite gives the same result on 5.x and on this branch: 370 passed and the same 3 failed both times, all three of which load external pages (it may visit external URLs and the two iframe tests). No test is added: TracingTest writes into the same directory, and under --parallel a test removing or populating it would race that test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: pest-plugin-browser 5.1.0: Trace::cleanup() makes a passing browser run exit 1 under failOnWarning

1 participant