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

Isolation via test scenarios #3644

Merged
merged 5 commits into from Aug 9, 2018
Merged

Conversation

greg-1-anderson
Copy link
Member

No description provided.

"unit": "phpunit --colors=always",
"functional": "./unish.phpunit.php",
"post-update-cmd": [
"create-scenario isolation --autoload-dir isolation --autoload-dir internal-copy --keep '\\(psr/log\\|consolidation/config\\|site-alias\\|var-dumper\\|symfony/finder\\|drupal-finder\\|path-util\\|sebastian/version\\|xhprof\\)' 'phpunit/phpunit:^5.5.4'",
Copy link
Member

Choose a reason for hiding this comment

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

these two calls happen after every drush update for all users (not just require-dev)? but test-scenarios package wont be present?

Copy link
Member Author

Choose a reason for hiding this comment

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

This won't run for site-local Drush users, but you are correct that this will cause problems for anyone who installs a global drush via Composer and then uses composer update --no-dev.

This typically isn't a problem for Robo applications, as folks can update their phar via the self:update command, and those who are not using the phar typically don't use --no-dev. Drush doesn't use this, though.

This won't cause a problem at the moment because Robo itself is including composer-test-scenarios in its 'require' section. I did this as a workaround for a similar problem with dependencies.io when I was trying to run composer update --no-dev in an attempt to only generate a PR when there were updates in the non-dev components. This didn't work out, though, so I'm probably going to move composer-test-scenarios back to require-dev in Robo eventually (even though it's very small).

Maybe what I need to do long-term is make composer-test-scenarios a Composer installer, and have it do nothing other than define a create-scenarios hook that runs from post-update-cmd when it is present. That would probably work.

Since the problem is kind of niche and is currently masked by Robo, are you okay with merging this as-is for now? You did approve the PR, but I wanted to double-check post explanation.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we lets proceed.

It would be helpful if you do the long term fix before moving composer-test-scenarios package to the require-dev section in Robo.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed

@greg-1-anderson greg-1-anderson merged commit 454a174 into master Aug 9, 2018
@weitzman weitzman deleted the isolation-via-test-scenarios branch December 13, 2019 15:44
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.

None yet

2 participants