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

Tests should not depend on external commands #4673

Closed
kirtangajjar opened this Issue Feb 8, 2018 · 5 comments

Comments

3 participants
@kirtangajjar
Contributor

kirtangajjar commented Feb 8, 2018

testRunMysqlCommandProcDisabled, testLaunchEditorForInputProcDisabled and testGetTempDir in test-utils.php and testLaunchProcDisabled in test-ee.php depends on eval-command.

This shouldn't be the case. If they have to, It's better to move such tests in a separate file(as it's done with test-bundled-commands.php)

@gitlost gitlost added this to the 2.0.0 milestone Feb 8, 2018

@gitlost

This comment has been minimized.

Contributor

gitlost commented Feb 8, 2018

Thanks @kirtangajjar . Not 100% sold on the philosophical argument but these are poor tests as they're never run on the phar so should have been fixed with #4597. Will refactor as behat tests.

@schlessera

This comment has been minimized.

Member

schlessera commented Feb 8, 2018

The main issue here is that you cannot run the "unit tests that are supposed to test the framework" without including the hidden dependency of the bundled command. So not only are they not "unit" tests (which, I know, is something WordPress does not really care about at all), but they also break when you just want to test your "package" in isolation.

@gitlost

This comment has been minimized.

Contributor

gitlost commented Feb 8, 2018

Well I find the whole bundled versus framework division artificial. But that should be part of the Great Package Debate following on from the last office hours...

@schlessera

This comment has been minimized.

Member

schlessera commented Feb 8, 2018

That division is not so artificial. I had to build a Composer plugin that splits 1 autoloader setup into two, just to make it work...

@kirtangajjar

This comment has been minimized.

Contributor

kirtangajjar commented Feb 9, 2018

@gitlost The thing is wp-cli is an excellent shell rather than being just a wp specific utility. So if we manage the code in such a way that it does not depend on wp specific commands, it will be very easy to fork the code and use it as a base to build another cli utility on top of it.

@schlessera schlessera modified the milestones: 2.0.0, 1.5.1 Apr 21, 2018

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