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

Change config file priority #26

Closed
wants to merge 1 commit into from

Conversation

@jordonbaade
Copy link

commented Aug 21, 2015

Changes config file priority so that configurationFile comes first if option.configurationFile is set, and gulp.src() second only if option.configurationFile is not set.

My solution to: https://github.com/laravel/elixir/issues/229

Change config file priority
Changes config file priority so that configurationFile comes first if option.configurationFile is set, and gulp.src() second only if option.configurationFile is not set.
@mikeerickson

This comment has been minimized.

Copy link
Owner

commented Aug 21, 2015

@jordonbaade Can you please provide a little bit more information about this issue. I hear what you are saying (about moving config file up on priority chain). but I am still unclear exactly what the exact issue is. I have to think about applying this to the core modules I use for all gulp-xxxx plugins (ie phpspec, behat, and codeception).

While it appears to be an isolated issue with regards to PHPUnit, I still want to know what is happening as gulp.src is basically an entrypoint into the plugin, but the actual code does not use the src files?

@jordonbaade

This comment has been minimized.

Copy link
Author

commented Aug 21, 2015

I'm new, so I'll try.

Basically the core issue I see is that this if statement:
https://github.com/mikeerickson/gulp-phpunit/blob/master/index.js#L180

Is impossible to enter because of this if statement:
https://github.com/mikeerickson/gulp-phpunit/blob/master/index.js#L175

Correct me if I'm wrong, but gulp.src() should always have an argument of files passed to it, otherwise it throws a fit. This means that a configurationFile if statement as second priority can never be entered as it stands.

Elixir is passing a string pattern for the test files from:
https://github.com/laravel/elixir/blob/master/tasks/phpunit.js#L21
to:
https://github.com/laravel/elixir/blob/master/tasks/shared/Tests.js#L13

It would seem this Elixir file was written prior to this commit which introduced pulling from gulp.src:
2b8ac6c

Jeffrey could probably change it so that there is a separate option in phpunit.js for the watch pattern, and update the gulp.src() to work with that last commit, but in the interest of keeping everything simple & flexible, and seeing that gulp-phpunit didn't seem (to me) to allow use of a configuration file through it's own option.configurationFile I thought this was a good route.

I also considered doing a extension check to see if it was an xml file, and if not go to a new priority that doesn't use -c (because when using -c phpunit is looking for the xml file, not the php test files.) but still pulls from gulp.src()

Happy to go any route, was bored and thought I'd try to contribute :)

TL:DR;

Elixir needs updated after a commit to gulp-phpunit, but gulp-phpunit option.configurationFile doesn't seem to work at all either.

@mikeerickson

This comment has been minimized.

Copy link
Owner

commented Aug 22, 2015

@jordonbaade I will get this cleared up this weekend for sure. Will aim to complete tomorrow, but have lots on my weekend plate, so Sunday at latest :)

@jordonbaade

This comment has been minimized.

Copy link
Author

commented Aug 22, 2015

Let me know if I can do anything to help out. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.