Test class with no tests gives non-zero stderr status, but no warning to stdout and shows all tests pass

[selfsimilar]

Dusk Version

7.6

Laravel Version

10.26.2

PHP Version

8.2.11

PHPUnit Version

10.0.7

Database Driver & Version

No response

Description

When a test class does not have any tests (e.g. commented out), dusk will show all other tests have passed, but return a non-zero console status which breaks CI flows.

This recently cost me a lot of time while I tried to determine why all my tests were reported as passing, but my CI pipeline was failing. If Dusk ever returns a non-zero response to stderr, there should be some indication in stdout as well, ideally pinpointing the issue so a user can attempt to fix it.

Dusk already does this e.g. if there is a completely empty test file it will issue a warning like

WARN Class EmptyTest cannot be found in /foo/bar/baz/tests/Browser/EmptyTest.php

However if a test class exists, yet has no tests (or all the tests are commented out), there is no warning printed to stdout.

Steps To Reproduce

$ composer create-project laravel/laravel example-app
$ cd example-app
$ composer install
$ composer require --dev laravel/dusk
$ php artisan dusk:install
$ php artisan dusk:chrome-driver
$ php artisan serve --port 80 &

NB - if port 80 is restricted you can edit APP_URL in .env to use port 8000

$ php artisan dusk
# All tests pass
$ echo $?
0

Now make a new test and comment out the body so no tests are run in the new test.

php artisan dusk:make EmptyTest

<?php

namespace Tests\Browser;

use Illuminate\Foundation\Testing\DatabaseMigrations;
use Laravel\Dusk\Browser;
use Tests\DuskTestCase;

class EmptyTest extends DuskTestCase
{
    /**
     * A Dusk test example.
     */
    // public function testExample(): void
    // {
    //     $this->browse(function (Browser $browser) {
    //         $browser->visit('/')
    //                 ->assertSee('Laravel');
    //     });
    // }
}

Run Dusk again and it will say all tests have passed, but return 1 at the console which will silently break a CI run.

$ php artisan dusk
# All tests pass
$ echo $?
1

[driesvints]

I realize this isn't ideal but this isn't a realistic scenario tbh. An empty test class should be removed.

We would appreciate a PR however to make this UX better.

[selfsimilar]

This is absolutely a realistic scenario - this is a thing that has happened. I have junior programmers (work studies) who commented out WIP tests. There's no documentation that this is a hazard. Especially when local runs of Dusk appear to be passing and it looks fine to send to a stage or dev environment via CI. "Why is my build failing? All test are passing but the build stops at Dusk!"

I will happily work towards a PR on this, but please don't tell me "I'm holding it wrong" or "this is an unrealistic scenario". No warnings on a non-zero stderr is just not ok.

I've already done a little digging, and this may be more of a PHPUnit issue rather than a Dusk issue; that I might agree to. But if you're saying this isn't a problem, it will be harder for me to convince any other project to fix this issue.

[selfsimilar]

This is a bug in Collision

Issue and a PR for the fix

nunomaduro/collision#293

nunomaduro/collision#294

[Pacific01]

This is absolutely a realistic scenario - this is a thing that has happened. I have junior programmers (work studies) who commented out WIP tests.

Indeed its a realistic scenario.

I was working on a GitHub Action pipeline, php artisan dusk:install adds tests/Browser/ExampleTest.php by default, there is a testBasicExample that looks for "Laravel" text on the home page, making the pipeline automatically fail.

My solution was to add/commit test/Browser/ExampleTest.php with no tests on It... And 4 hours later of debugging sail dusk and asking myself why echo $? returns 1 if every test is on green? A real pain in the ***

[selfsimilar]

@Pacific01 - I dug in on this and discovered this is a downstream error from the Collision package that Dusk relies on. I've submitted a patch, but it is still unmerged. You could take that patch, or perhaps comment on the PR to see if that speeds the acceptance of the patch.