Skip to content
This repository has been archived by the owner on Aug 8, 2024. It is now read-only.

Try Circle 2.0. #107

Merged
merged 19 commits into from
Sep 13, 2017
Merged

Try Circle 2.0. #107

merged 19 commits into from
Sep 13, 2017

Conversation

greg-1-anderson
Copy link
Member

No description provided.


- run:
name: dependencies
command: ./.circleci/scripts/00-set-enviornment
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe enviornment should be environment?

stevector added a commit to pantheon-systems/example-wordpress-composer that referenced this pull request Sep 12, 2017

# Create a new multidev site to test on
terminus -n env:wake "$TERMINUS_SITE.dev"
terminus -n build:env:create "$TERMINUS_SITE.dev" "$TERMINUS_ENV" --yes --clone-content --notify="$NOTIFY"
Copy link
Contributor

Choose a reason for hiding this comment

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

@greg-1-anderson isn't this always creating the multidev from dev/master on Pantheon, rather than the code of the branch in the external Git repository?

Copy link
Member Author

Choose a reason for hiding this comment

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

build:env:create will first create a new multidev environment from the specified base environment (here, the dev environment), and then will overlay the code found at the cwd in the new multidev.

If a multidev of the requested name already exists, then it will be re-used, and again, the code from the cwd will be pushed to this multidev.

Copy link
Contributor

@ataylorme ataylorme Sep 13, 2017

Choose a reason for hiding this comment

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

and then will overlay the code found at the cwd in the new multidev.

Ah, I was forgetting about that part.

Copy link
Contributor

Choose a reason for hiding this comment

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

It still seems --notify="$NOTIFY" is running on each commit, even after a multidev exists. Do you think it makes sense to modify build:env:create so that --notify="$NOTIFY" only fires if the multidev doesn't already exist?

Not having multiple notifications is my only motivation for using this conditional in example-wordpress-composer rather than just build:env:create

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's put that logic in the Build Tools plugin itself: pantheon-systems/terminus-build-tools-plugin#105

@greg-1-anderson
Copy link
Member Author

The odd thing here is that the test failed when I ran behat via a Composer script instead of direct execution. Not sure if that was just a fluke.

@greg-1-anderson
Copy link
Member Author

So, at this point the contents of the .circleci/scripts/pantheon folder is just a bunch of scripts that for the most part just call terminus a lot. I think these could move to a separate Composer project that we semver at ^1 at first, and move to ^2 if/when we need to make any breaking changes to the test steps.

The 00-set-environment script is tied to the docker container that is being used; perhaps it could be moved into the docker container itself. Once we support multiple test platforms, we might need separate versions of this script for circle, travis, etc. (if we decide to use docker on travis).

@greg-1-anderson
Copy link
Member Author

Things that the WordPress example project is doing that we do not do here:

  • Skips creating a multidev and running behat when there is no PR
  • Backup the site before the tests, and restore it after the tests
  • Run terminus wp once prior to running behat to init ssh
  • Delete and re-create the admin user

Things that the Drupal example project is doing that is not needed by WordPress:

  • drush updatedb
  • drush config-import

Also, both projects have different behat parameters.

Perhaps these custom steps can be factored out as composer scripts, and customized in the composer.json file for the respective framework.

@ataylorme
Copy link
Contributor

Backup the site before the tests, and restore it after the tests

I think this is important as Behat tests will make changes to content. After tests pass it would be ideal to have a multidev ready for stakeholder review. Without backup/restore of the database we would be showing off a multidev with Behat changes applied, rather than reflecting the actual content.

An alternative might be to re-clone the database from another environment and then do a config import but I think a restore is faster/easier.

@ataylorme
Copy link
Contributor

Backup the site before the tests, and restore it after the tests

This helped me avoid timeouts and other Terminus errors when using the wp-cli drive via Terminus for WordHat

@ataylorme
Copy link
Contributor

Backup the site before the tests, and restore it after the tests

I don't think a test admin user should be left around after testing is complete.

@stevector
Copy link
Contributor

Things that the Drupal example project is doing that is not needed by WordPress:

drush updatedb
drush config-import

Those are CMS specific details and could go in Quicksilver scripts in example-drops-8-composer.

The differences in the WordPress repo are not CMS specific but developer workflow decisions. They should be consistent between our example repos IMO.

@ataylorme
Copy link
Contributor

Things that the Drupal example project is doing that is not needed by WordPress: drush updatedb

It might be necessary to run the wp-cli core update-db after a WordPress core or plugin is updated and it won't hurt to always run it.

@ataylorme
Copy link
Contributor

The differences in the WordPress repo are not CMS specific but developer workflow decisions. They should be consistent between our example repos IMO.

+1 to coming to a consensus

@greg-1-anderson
Copy link
Member Author

greg-1-anderson commented Sep 13, 2017

Could I get a +1 on merging this PR as-is?

Next step would be making QS scripts for the CMS-specific updatedb / config-import, and then tackle workflow unification decisions in follow-on issues.

@ataylorme
Copy link
Contributor

You've got my 👍 as long as we open those other issues and don't lose track of these conversations/additional work

@greg-1-anderson
Copy link
Member Author

OK, I will make separate issues in this project for the issues raised above.

@greg-1-anderson
Copy link
Member Author

Created #111 to keep an index to issues with active workflow unification discussion. If you create an issue for one of these topics, link to it here. I'll eventually start separate issues for each if someone else doesn't get to them first.

@greg-1-anderson
Copy link
Member Author

Also created #112 for the Quicksilver script conversion.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants