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

Exit code 0 on failed tests #690

Closed
petermein opened this issue Nov 6, 2019 · 4 comments
Closed

Exit code 0 on failed tests #690

petermein opened this issue Nov 6, 2019 · 4 comments

Comments

@petermein
Copy link
Contributor

  • Dusk Version: 5.6
  • Laravel Version: 6.0.4
  • PHP Version: 7.2.23
  • Database Driver & Version: multiple
  • PHPUnit: 8.4.2
  • Environment: CircleCi and Local

Description:

When looking at our CircleCi tests and local tests, we noticed even when test failed the exit code returned from the php artisan dusk command is still 0. This means even if you publish test results the test will be marked as successfull

Steps To Reproduce:

  • Run in bash php artisan dusk && echo "$?" || echo "$?" (with atleast 1 failing test)
    This will display an exit code of 0 in our tests.

Time: 9.05 seconds, Memory: 44.25 MB

There was 1 error:

1) Tests\Browser\Tests\TwoFactorLoginTest::testTwoFactorLogin
Facebook\WebDriver\Exception\TimeOutException: Waited 5 seconds for callback.

/var/www/vendor/laravel/dusk/src/Concerns/WaitsForElements.php:261
/var/www/vendor/laravel/dusk/src/Concerns/InteractsWithElements.php:296
/var/www/tests/Browser/Tests/TwoFactorLoginTest.php:36
/var/www/vendor/laravel/dusk/src/Concerns/ProvidesBrowser.php:67
/var/www/tests/Browser/Tests/TwoFactorLoginTest.php:43

ERRORS!
Tests: 1, Assertions: 0, Errors: 1.
0
@mnabialek
Copy link
Contributor

It can be true. I noticed this yesterday when upgraded Dusk to latest.

@petermein
Copy link
Contributor Author

petermein commented Nov 6, 2019

There was a return statement missing in the last update when adding the SIGINT.
I've added a PR #691

Maybe testing the error code could be added as a test?

@petermein
Copy link
Contributor Author

PR #691 is merged @driesvints could you create a new version and push it to packagist?

@driesvints
Copy link
Member

@petermein new version will be released tomorrow on release day.

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

No branches or pull requests

3 participants