Skip to content

Conversation

@greg-1-anderson
Copy link
Member

This PR changes the PHP version constraint to 7.0 in the platform section. This will prevent problems for folks who would otherwise be unable to run composer install on a PHP 7.0 system, should the composer.lock file be generated by someone running PHP 7.1.

@greg-1-anderson
Copy link
Member Author

@ataylorme Do you have any idea why this test is failing?

@stevector
Copy link
Collaborator

My guess for the cause of the fail is that CircleCI kicks off immediately upon a commit being pushed. The build_and_deploy job then runs with the CIRCLE_PULL_REQUEST variable empty. By the time the behat_tests job runs a PR has been created and CIRCLE_PULL_REQUEST is populated. behat_tests then assumes that a multidev was created for it to use.

Options for resolving

  • Switch to only having CircleCI run on Pull Request, not every push. This may be a non-starter as it is not CircleCI's default behavior and we'd need to change the build plugin to set this config upon repo creation. And plenty of teams will want some tests run on push.
  • Somehow pass from build_and_deploy to behat_tests whether an env was created.
  • Have behat_tests do it's own multidev creation if one does not exist.
  • Others?

@stevector
Copy link
Collaborator

And now we're failing with

 Your WP-CLI is too old; version 1.3.0 or newer is required.

I think that is coming from the latest version of WordHat: https://github.com/paulgibbs/behat-wordpress-extension/releases/tag/v0.7.1 We should probably explicitly require an older version of WordHat until Pantheon is running 1.3.0 or higher.

This was referenced Sep 8, 2017
@ataylorme
Copy link
Contributor

We should probably explicitly require an older version of WordHat until Pantheon is running 1.3.0 or higher.

@stevector we should use 0.7.1 (see #13). It was tagged before the wp-cli requirement was bumped.

My guess for the cause of the fail is that CircleCI kicks off immediately upon a commit being pushed. The build_and_deploy job then runs with the CIRCLE_PULL_REQUEST variable empty. By the time the behat_tests job runs a PR has been created and CIRCLE_PULL_REQUEST is populated. behat_tests then assumes that a multidev was created for it to use.

File this under other ideas but what if we don't explicitly start the Behat job but rather fire it via API from build_and_deploy if a multidev is created/pushed to (e.g. we're on a pull request). The API job trigger doc outlines how to do this.

@stevector
Copy link
Collaborator

0.7.1 Required here: #37

Triggering jobs via curls from other jobs strikes me as pretty inelegant. But then again, so is running behat_tests on every single build only to quit out of it for non-PRs. @greg-1-anderson, what do you think?

Would taking behat_tests out of the Workflow and only triggering it via curl mean that we wouldn't see it in the Circle Workflow diagrams?

@greg-1-anderson
Copy link
Member Author

Yes, we can make behat_tests echo a no-op warning and then exit cleanly if the current test is not a pull request. We can detect the pull request status using environment variables that circle sets. I'll upgrade Pantheon to wp-cli 1.3.0 pretty soon (Monday, I'd guess); either leave this broken until then, or pin to the older version of WordHat as suggested.

For my ideas on how to handle this in the future, see pantheon-systems/example-drops-8-composer#103.

@stevector
Copy link
Collaborator

@greg-1-anderson I added a readme file change for PHP version. That has triggered tests again.

@stevector
Copy link
Collaborator

I've made a follow up issue for investigating the logic around Behat running or not: #40 I think we should merge this as is. I see @ataylorme has approved it. Merging now.

@stevector stevector merged commit dec46f1 into master Sep 12, 2017
@stevector stevector deleted the php-70 branch September 13, 2017 13:47
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.

4 participants