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

After install event breaking automated testing #37

Closed
davist11 opened this issue Jul 8, 2021 · 9 comments
Closed

After install event breaking automated testing #37

davist11 opened this issue Jul 8, 2021 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@davist11
Copy link
Contributor

davist11 commented Jul 8, 2021

We have automated testing setup on our Craft + Commerce install. Once we added this plugin, the test suite no longer runs completely (it doesn't install other plugins we have installed). I commented this out just to test:

Event::on(
Plugins::class,
Plugins::EVENT_AFTER_INSTALL_PLUGIN,
function (PluginEvent $event) {
if ($event->plugin === $this) {
$this->onAfterInstall();
}
}
);

And the suite runs correctly then. I'm not sure the best solution to fix this. Should this be moved up to the top of the method?

craft-avatax/src/Avatax.php

Lines 307 to 309 in 67c93a8

if (Craft::$app->getRequest()->getIsConsoleRequest()) {
return;
}

Or maybe check to see if the env is test?

Craft Pro 3.6.11.1
Avatax 2.1.5

@siffring siffring added the bug Something isn't working label Jul 8, 2021
@davist11
Copy link
Contributor Author

👋

Any potential update here? Been holding off merging this plugin in. Thanks!

@imagehat
Copy link
Collaborator

@davist11 - Admittedly I haven't done a deep dive into Craft's testing, but are there any particular errors you're getting that may help track it down?

I'm hesitant to move up the console request check because I think we'd still want to install the Tax Category and override Fields if you're installing the plugin from the console. That check is just to prevent the CP redirect if running in the console.

Are you defining CRAFT_TESTS_PATH in tests/_bootstrap.php? One thought I had was to check for that constant to see if it's running in test mode and then just return. I'm not sure if there's a better way but that might be an easy one to try.

@davist11
Copy link
Contributor Author

There isn't really a great error to send you down a path because I just get a generic:

In Schema.php line 678:
                                                                               
  SQLSTATE[42S02]: Base table or view not found: 1146 Table 'client_test.<nil>_n  
  avigation_navs' doesn't exist                                                
  The SQL being executed was: SELECT *                                         
  FROM `<nil>_navigation_navs`                                                 
  WHERE `<nil>_navigation_navs`.`dateDeleted` IS NULL                          
  ORDER BY `sortOrder`                                                         
                                                                               

In Command.php line 1299:
                                                                               
  SQLSTATE[42S02]: Base table or view not found: 1146 Table 'client_test.<nil>_n  
  avigation_navs' doesn't exist   

Something in that onAfterInstall() method is preventing other plugins from being installed, but I'm not sure exactly which part. I can dig in and start doing more debugging if that's helpful to find out exactly where it's halting the installation of other plugins.

Are you defining CRAFT_TESTS_PATH in tests/_bootstrap.php

Yep, we have all these constants set in our bootstrap:

define('CRAFT_TESTS_PATH', __DIR__);
define('CRAFT_STORAGE_PATH', __DIR__ . '/_craft/storage');
define('CRAFT_TEMPLATES_PATH', __DIR__ . '/_craft/templates');
define('CRAFT_CONFIG_PATH', __DIR__ . '/_craft/config');
define('CRAFT_MIGRATIONS_PATH', __DIR__ . '/_craft/migrations');
define('CRAFT_TRANSLATIONS_PATH', __DIR__ . '/_craft/translations');
define('CRAFT_VENDOR_PATH', dirname(__DIR__) . '/vendor');

Maybe a FR for Craft would be to have some sort of boolean for when the test suite is running. Another option would be checking to see if the environment == test?

@imagehat
Copy link
Collaborator

I see. So it's breaking the next plugin install and maybe causes an error because the Navigation plugin didn't run it's migrations or something.

I wasn't sure if the environment was alway set to test when testing but if that's the case maybe we try adding something like:

if (Craft::$app->config->env === 'test') {
  return;
}

... at the top of this function to just bail:

craft-avatax/src/Avatax.php

Lines 199 to 204 in 67c93a8

/**
* Raised after the plugin is installed.
* Create the AvaTax Tax category and product fields.
*/
public function onAfterInstall()
{

@davist11
Copy link
Contributor Author

I think so, you can see in the test setup that is sets a default env value of test

https://github.com/craftcms/cms/blob/develop/src/test/TestSetup.php#L262-L281

And yeah, I think adding a check like that makes sense. Want me to manually add it and test? Would be happy to submit a PR.

@imagehat
Copy link
Collaborator

Good deal. Yeah if you can test it that would be awesome. Either a PR and I'll merge it or just let me know if the conditional works and I can get it added in and bump the version.

@davist11
Copy link
Contributor Author

Success, test suite passes! #38

@imagehat
Copy link
Collaborator

Thanks @davist11! Merged and version bumped.

@davist11
Copy link
Contributor Author

Awesome, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants