Add Bitbucket Pipelines by tfirdaus · Pull Request #118 · wp-cli/scaffold-command

tfirdaus · 2018-01-22T04:45:12Z

Adding Bitbucket Pipelines as one of the CI providers (Resolve #9), allowing users to specify bitbucket on the wp scaffold command. For examples:

Scaffold Plugin Test:

wp scaffold plugin-tests hello-world --ci=bitbucket

Scaffold Theme Test:

wp scaffold theme-tests p2child --ci=bitbucket

Example repositories with a working pipelines can be found at:

Add bitbucket to the command options list and fix the alignment

I wasn't aware that the README is auto-generated.

gitlost

Thanks very much for the PR @tfirdaus and apologies tor the delay in reviewing. On (albeit very limited) testing by me it works great. Was wondering about the reference to tfirdaus/wp-docklines in the template though?

gitlost · 2018-01-27T22:09:56Z

templates/plugin-bitbucket.mustache

+pipelines:
+  default:
+    - step:
+        image: tfirdaus/wp-docklines:php5.6-fpm-alpine


It'd be good to use something more neutral here (and passim) I think rather than your personal stuff! Is there a way to do that??

Hi, @gitlost
Thank you for reviewing the pull request.

Yes, that's possible. However, in the case of Bitbucket Pipelines, I'm afraid doing so might harm users sitting on the Free plan as Bitbucket only allocates 50 minutes of build per month on the Free plan. Bitbucket suggests creating our own Docker images that are ready to go as setting up dependencies and tools during the build will be time-consuming.

Here is what that says in one of their articles Tips for scripting tasks with Bitbucket Pipelines

Installing the dependencies necessary for your pipeline to run can be time-consuming. You could save a lot of running time by creating your own Docker image with the basic tools and packages required to build and test your application.

Hence the reason for those personal images 🙂

I'm open to suggestion; if including personal images is not allowed I'm more than happy to change it to a neutral image.

Hi @tfirdaus !

Bitbucket suggests creating our own Docker images that are ready to go as setting up dependencies and tools during the build will be time-consuming.

Ah I see, ta. (Forgive my ignorance, I know next to nothing about Docker also and only just noticed your images here https://hub.docker.com/r/tfirdaus/wp-docklines/)

So perhaps the way to go in the future might be to register wp-cli or similar on Docker Hub and upload the images there. But in the meantime we could use your (oh so personal!) account.

Thanks for the feedback, would like to get input from @schlessera on it - though we're about to release WP-CLI 1.5.0 on Tuesday so may not be in touch immediately...

Understood. I'm happy to transfer the Docker image ownership to WP-CLI official if that's necessary 🙂as the image might also be useful for GitlabCI or CircleCI scaffold command.

@tfirdaus I understand the reason why you added the (personal) image to the script. However, I don't want to include such a custom image in this script for the following reasons:

Providing an official WP-CLI image means that the WP-CLI maintainers need to indefinitely maintain this image. I consider this is out of scope for the WP-CLI project.

Providing your personal image is a potential security/trust issue. We would basically endorse an unknown black box image. I don't feel comfortable doing this, not because I don't trust you as a person, but because I think that this is just not a good idea in principle. We have no control over that image, so must assume it cannot be trusted.

The scaffolding tools are meant to provide a starting point. Having a basic BB Pipelines setup that has a limitation on the free plan is perfectly okay. It still helps people to get started quickly, and they are meant to do adjustments to fit their needs either way.

Given the above, would you be willing to change the PR so that it doesn't include a pre-built image, but rather compiles a minimum set of dependencies on the fly?

Understood and that totally makes sense. I'll rewrite the Pipelines config to the minimum :) Thank you for your feedback @schlessera

This reverts commit 664df92. # Conflicts: # features/scaffold-theme-tests.feature

This reverts commit 4a69998.

This reverts commit 62e24a4.

This reverts commit a926994.

This reverts commit 7eab5f0.

This reverts commit 0312d52.

This reverts commit e31f6d5. # Conflicts: # features/scaffold-theme-tests.feature

tfirdaus · 2018-02-18T09:56:12Z

@schlessera & @gitlost I apologize. I think I messed up the commits for this pull requests.
Should I close this PR and create a new one from another branch in the fork?

It appears there might be a compatibility issue with the test suite running on PHP 7.2. All tests went through except for PHP 7.2 https://travis-ci.org/tfirdaus/scaffold-command/builds/342950123. The error says:

Fatal error: Class PHPUnit_Util_Test may not inherit from final class (PHPUnit\Util\Test) in /tmp/behat-wordpress-tests-lib/includes/phpunit6-compat.php on line 18

gitlost · 2018-02-18T18:26:48Z

Should I close this PR and create a new one from another branch in the fork?

It's fine as it is @tfirdaus, thanks.

It appears there might be a compatibility issue with the test suite running on PHP 7.2.

Yes, see https://core.trac.wordpress.org/ticket/43218 for the report. I think for the moment we can just explicitly use phpunit 6.5.6 for PHP 7.2 - will do a PR for this.

tfirdaus · 2018-02-19T01:54:20Z

Thanks @gitlost 💯
I'll kick in another commit to trigger the build once #125 is merged :)

schlessera · 2018-02-20T08:58:35Z

@tfirdaus We've include a work-around for now. Can you merge latest master? That should fix the tests.

tfirdaus · 2018-02-22T01:36:34Z

Thanks @schlessera 👍 , I'll try to do it on this weekend :)

It’s strange that one failed and the other one passed of the same build. (Build wp-cli#13 passed) https://travis-ci.org/tfirdaus/scaffold-command/builds/345534407 (Build wp-cli#232 failed) https://travis-ci.org/wp-cli/scaffold-command/builds/345534415

tfirdaus · 2018-02-24T04:58:03Z

@gitlost and @schlessera thanks, The issue on PHP 7.2 is fixed. All tests are passed as it seems now 💯

gitlost

Thanks @tfirdaus , just a nitpick regarding phpcs induced change.

gitlost · 2018-02-25T16:29:38Z

templates/plugin-bootstrap.mustache


 if ( ! file_exists( $_tests_dir . '/includes/functions.php' ) ) {
-	echo "Could not find $_tests_dir/includes/functions.php, have you run bin/install-wp-tests.sh ?" . PHP_EOL;
+	esc_html( "Could not find $_tests_dir/includes/functions.php, have you run bin/install-wp-tests.sh ?" ) . PHP_EOL;


I think to get around the incorrect phpcs echo warning probably best to do

fwrite( STDERR, "Could not find '$_tests_dir/includes/functions.php', have you run bin/install-wp-tests.sh ?" . PHP_EOL );

here, as esc_html() not appropriate for shell output.

Hi, @gitlost
As I recall I did not make this change 🙁.
I'm wondering if it was added when I messed up the commit (I'm not quiet certain on this).

I also think that using esc_html function does not make any sense, as it won't be loaded upon executing install-wp-tests.sh, and looking at the master (https://github.com/wp-cli/scaffold-command/blob/master/templates/plugin-bootstrap.mustache#L15), it should be just echo.

Do you think we can revert it back to echo?

Ah okay, it somehow got added in the melee here #118 (commits).

Do you think we can revert it back to echo?

Yes, that's the simplest thing to do, as contrary to what I said above phpcs doesn't seem to object to it anyway, so please just do that, thanks!

gitlost · 2018-02-27T00:35:00Z

Thanks @tfirdaus, good stuff! I suppose we could iterate on this to include the testing of different WP_VERSIONs like in plugin-travis.mustache#L25-L28 sometime in the future...

tfirdaus · 2018-02-27T01:36:44Z

Definitely, I think we could do it in the future 👍
@gitlost & @schlessera, thank you for reviewing this PR.

Add Bitbucket Pipelines

tfirdaus added 5 commits January 22, 2018 10:20

Init Bitbucket Pipelines

b2c016c

Add “bitbucket” in the command list

36090e8

Add “bitbucket” in the command list

8117f1a

Update README.md

a7e6730

Add bitbucket to the command options list and fix the alignment

Revert previous README.md changes

d156d4c

I wasn't aware that the README is auto-generated.

gitlost reviewed Jan 27, 2018

View reviewed changes

gitlost added command:scaffold-plugin-tests Related to 'scaffold plugin-tests' command command:scaffold-theme-tests Related to 'scaffold theme-tests' command labels Jan 27, 2018

tfirdaus and others added 16 commits February 18, 2018 15:33

Update Pipelines config with PHP Docker image

9230cb3

Update the Bitbucket Pipelines test

e31f6d5

Filter theme_root in tests/bootstrap.php.

664df92

Need to use install-wp-tests.sh before running phpunit.

4a69998

fix wpcs in child theme generation

62e24a4

fix phpcs warnings plugin

a926994

Just make plugin.mustache change for the mo.

7eab5f0

v1.1.2 prerelease: refresh test suite, require wp-cli ^1.5.

0312d52

Realign Bitbucket theme tests feature

69e2f08

Revert "Filter theme_root in tests/bootstrap.php."

1f4ba8f

This reverts commit 664df92. # Conflicts: # features/scaffold-theme-tests.feature

Revert "Need to use install-wp-tests.sh before running phpunit."

6fa54f5

This reverts commit 4a69998.

Revert "fix wpcs in child theme generation"

c9a8231

This reverts commit 62e24a4.

Revert "fix phpcs warnings plugin"

f3b9ce4

This reverts commit a926994.

Revert "Just make plugin.mustache change for the mo."

d986b02

This reverts commit 7eab5f0.

Revert "v1.1.2 prerelease: refresh test suite, require wp-cli ^1.5."

357c004

This reverts commit 0312d52.

Revert "Update the Bitbucket Pipelines test"

e0ab7c8

This reverts commit e31f6d5. # Conflicts: # features/scaffold-theme-tests.feature

tfirdaus added 3 commits February 18, 2018 20:40

Update Bitbucket test

7fcbc7e

Fix the undefined {THEME_DIR}

0d0cd21

Fix plugin path

05b05fe

gitlost mentioned this pull request Feb 18, 2018

Use phpunit 6.5.6 for PHP 7.2 to get around core test incompat. #125

Merged

tfirdaus added 2 commits February 24, 2018 10:11

Merge branch 'master' of https://github.com/wp-cli/scaffold-command

9446fc0

Kick in another commit

238d6a0

It’s strange that one failed and the other one passed of the same build. (Build wp-cli#13 passed) https://travis-ci.org/tfirdaus/scaffold-command/builds/345534407 (Build wp-cli#232 failed) https://travis-ci.org/wp-cli/scaffold-command/builds/345534415

schlessera requested a review from a team February 25, 2018 09:29

gitlost suggested changes Feb 25, 2018

View reviewed changes

gitlost mentioned this pull request Feb 26, 2018

Switch CircleCI template to CircleCI 2.0. #115

Merged

Revert back to “echo”

c783ac9

gitlost approved these changes Feb 27, 2018

View reviewed changes

gitlost merged commit 2a3fa7a into wp-cli:master Feb 27, 2018

danielbachhuber pushed a commit that referenced this pull request Nov 18, 2022

Merge pull request #118 from tfirdaus/master

61edaec

Add Bitbucket Pipelines

Conversation

tfirdaus commented Jan 22, 2018

Uh oh!

gitlost left a comment

Choose a reason for hiding this comment

Uh oh!

gitlost Jan 27, 2018

Choose a reason for hiding this comment

Uh oh!

tfirdaus Jan 28, 2018

Choose a reason for hiding this comment

Uh oh!

gitlost Jan 28, 2018

Choose a reason for hiding this comment

Uh oh!

tfirdaus Jan 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schlessera Feb 14, 2018

Choose a reason for hiding this comment

Uh oh!

tfirdaus Feb 15, 2018

Choose a reason for hiding this comment

Uh oh!

tfirdaus commented Feb 18, 2018

Uh oh!

gitlost commented Feb 18, 2018

Uh oh!

tfirdaus commented Feb 19, 2018

Uh oh!

schlessera commented Feb 20, 2018

Uh oh!

tfirdaus commented Feb 22, 2018

Uh oh!

tfirdaus commented Feb 24, 2018

Uh oh!

gitlost left a comment

Choose a reason for hiding this comment

Uh oh!

gitlost Feb 25, 2018

Choose a reason for hiding this comment

Uh oh!

tfirdaus Feb 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gitlost Feb 26, 2018

Choose a reason for hiding this comment

Uh oh!

gitlost commented Feb 27, 2018

Uh oh!

tfirdaus commented Feb 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tfirdaus Jan 28, 2018 •

edited

Loading

tfirdaus Feb 26, 2018 •

edited

Loading