Skip to content

Conversation

@matthewswain
Copy link

Hi @nunomaduro ,

Thanks for maintaining this project! Hopefully I have explained my change in enough detail below but please let me know if it needs further explanation or changes.


The TestCommand uses ignoreValidationErrors to allow it to accept and pass through arbitrary options to phpunit/paratest. For example it does not define --stop-on-error but can pass it through to phpunit.

A side effect of this is that the order of options matters, e.g.

php artisan test -p --recreate-databases --stop-on-error does recreate databases, while

php artisan test -p --stop-on-error --recreate-databases does not.

This appears to be because when the validator hits an option it does not know about, it stops processing and any subsequent options cannot be resolved via $this->option.

It looks like this was previously handled when passing through options to phpunit/paratest but was not handled when setting environment variables, this change fixes that.

The TestCommand uses ignoreValidationErrors to allow it to accept and
pass through arbitrary options to phpunit/paratest. For example it does
not define `--stop-on-error` but can pass it through to phpunit.

A side effect of this is that the order of options matters, e.g.
`php artisan test -p --recreate-databases --stop-on-error` does recreate
databases, while
`php artisan test -p --stop-on-error --recreate-databases` does not.

This appears to be because when the validator hits an option it does not
know about, it stops processing and any subsequent options cannot be
resolved via $this->option.

It looks like this was previously handled when passing through options
to phpunit/paratest but was not handled when setting environment
variables, this change fixes that.
Copy link

@iBotPeaches iBotPeaches left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea if this fixes it, but I just spent a chunk of time painfully wondering why this was occurring. Trying to find TestCommand in Laravel to learn it was part of Collision, to then this PR.

Thanks for fix - hope it gets merged.

@johnkary
Copy link

I also found this issue today and wondered why it wasn't working, then found my way here.

I manually applied this PR to my dev environment (still using Laravel v10.48.29 and nunomaduro/collision v7.11.0) but it did allow my command to run.

Before this PR:

php artisan test --no-progress --parallel
PHPUnit 10.5.45 by Sebastian Bergmann and contributors.

Unknown option "--parallel"

After this PR:

(Paratest correctly executed and did not show any progress)

This anecdote doesn't prove this PR works for all other options combinations, but appears solved for the above scenario with Paratest.

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.

3 participants