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

Use @wordpress/env (wp-env) for local dev environment #5612

Closed
wants to merge 1 commit into from

Conversation

swissspidy
Copy link
Collaborator

@swissspidy swissspidy commented Dec 9, 2020

Summary

Instead of using a custom Docker setup for our local development environment, this uses the pre-existing @wordpress/env package dedicated for this task.

This reduces the maintenance burden and simplifies setup for contributors. Over time, more and more things can be moved to run within wp-env, eventually removing the need to manually install PHP, Composer, and PHPUnit, for example.

Relevant Technical Choices

To-do

Upstream blockers

WordPress/gutenberg#27783
WordPress/gutenberg#28703 / WordPress/gutenberg#29430
WordPress/gutenberg#29323
WordPress/gutenberg#28201
WordPress/gutenberg#27763
WordPress/gutenberg#20569

So most of https://github.com/WordPress/gutenberg/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3A%22%5BPackage%5D+Env%22 basically

Additional Context

User-facing changes

N/A

Testing Instructions

  1. npm run wp-env start -- --update --debug
  2. npm run test:e2e:prepare
  3. npm run test:e2e

Fixes #2521

@swissspidy swissspidy added Type: Infrastructure Changes impacting testing infrastructure or build tooling Pod: WP & Infra labels Dec 9, 2020
@google-cla google-cla bot added the cla: yes label Dec 9, 2020
@spacedmonkey

This comment has been minimized.

@spacedmonkey
Copy link
Contributor

Seems like they are using composer v2 in the WordPress container. Related #5083

@spacedmonkey
Copy link
Contributor

This error keeps happening.

node-pre-gyp
WARN Using request for node-pre-gyp https download
node-pre-gyp
WARN Tried to download(404): https://axonodegit.s3.amazonaws.com/nodegit/nodegit/nodegit-v0.26.5-node-v83-linux-x64.tar.gz 
node-pre-gyp WARN Pre-built binaries not found for nodegit@0.26.5 and node@14.15.1 (node-v83 ABI, glibc) (falling back to source compile with node-gyp)
/bin/sh: 1: krb5-config: not found
gyp: Call to 'krb5-config gssapi --libs' returned exit status 127 while in binding.gyp. while trying to load binding.gy

Once this is fixed, I think that it lots more things will start working.

@github-actions

This comment has been minimized.

@swissspidy
Copy link
Collaborator Author

You also need to fix npm run env:stop and npm run env:start. But I think you can just replace npm run wp-env start. You need to active plugins, setup consts, create users and import media. I think there still needs to be a start.sh file.

The wp-env.json config file takes care of all of this, except for adding users. But I think we could also easily just do this on CI

@spacedmonkey

This comment has been minimized.

@spacedmonkey
Copy link
Contributor

We can update this now with 3.0.0 - https://www.npmjs.com/package/@wordpress/env 🎉

@swissspidy swissspidy added the Status: Blocked On hold for the time being label Dec 23, 2020
@spacedmonkey
Copy link
Contributor

What is happening with this PR?

@swissspidy
Copy link
Collaborator Author

@spacedmonkey Thanks for asking! I think we're getting there, but I'm afraid we can only use it for the E2E tests for now. I was hoping we could use wp-env for more stuff from the beginning, but there are some upstream blockers that need to be fixed first.

Fo now, I just need to figure out why the E2E tests are failing, then this should be good to go.

PHP unit tests, PHP lints, and more, can then be addressed in future PRs.

@spacedmonkey
Copy link
Contributor

but I'm afraid we can only use it for the E2E tests for now.

If we are just using it from E2E tests, that I am not sure I see the benefit of using it all. If anything, it makes it hard to replicate if there is an error in e2e, as the envs are even more different from local / GHA.

Can you provide more detail on these upstream blockers? Maybe we could work on these together to get the unblocked and so we can start using this package.

@swissspidy
Copy link
Collaborator Author

but I'm afraid we can only use it for the E2E tests for now.

If we are just using it from E2E tests, that I am not sure I see the benefit of using it all. If anything, it makes it hard to replicate if there is an error in e2e, as the envs are even more different from local / GHA.

To clarify, we'd be able to use it for local and E2E tests. But not for PHP unit tests or running any composer commands through docker

Can you provide more detail on these upstream blockers? Maybe we could work on these together to get the unblocked and so we can start using this package.

There's some discussion at WordPress/gutenberg#27783. In WordPress/gutenberg#27783 (comment) I posted a suggestion for a possible solution. Shouldn't be too difficult to implement, and maybe we could even patch it in our repository as well so that we don't have to wait for the fix to be published on npm.

@spacedmonkey
Copy link
Contributor

So the issue seems to be running in PHP 5.6? Is that correct. As much as I don't love this idea, could we not just disable tests running in that version of PHP until we can patch this / get a fix merged?

@swissspidy
Copy link
Collaborator Author

So the issue seems to be running in PHP 5.6? Is that correct. As much as I don't love this idea, could we not just disable tests running in that version of PHP until we can patch this / get a fix merged?

It affects anything below 7.2, so 5.6, 7.0, and 7.1.

That said, I've now added a patch for the wp-env package to work around this

@swissspidy swissspidy removed the Status: Blocked On hold for the time being label Jan 5, 2021
@swissspidy swissspidy changed the title [EXPERIMENT] Use @wordpress/env (wp-env) for local dev environment Use @wordpress/env (wp-env) for local dev environment Jan 6, 2021
@swissspidy swissspidy force-pushed the try/wp-env branch 6 times, most recently from 8573bec to c5f6e32 Compare January 8, 2021 15:56
@google-cla

This comment has been minimized.

@google-cla google-cla bot removed the cla: yes label Jan 8, 2021
@swissspidy swissspidy force-pushed the try/wp-env branch 3 times, most recently from 689d1ca to b8728c5 Compare January 11, 2021 14:01
@spacedmonkey
Copy link
Contributor

Screenshot 2021-01-14 at 13 34 39

On npm i, seeing some error messages in the logs.

@swissspidy
Copy link
Collaborator Author

On npm i, seeing some error messages in the logs.

What's your node & npm version?

@spacedmonkey
Copy link
Contributor

jonny@macbook-pro web-stories-wp (try/wp-env ✗)]$ npm --version
6.14.11
[jonny@macbook-pro web-stories-wp (try/wp-env ✗)]$ nvm --version
0.37.2
[jonny@macbook-pro web-stories-wp (try/wp-env ✗)]$

@spacedmonkey
Copy link
Contributor

Screenshot 2021-01-14 at 16 47 16

Unable to run phpunit tests locally.

Screenshot 2021-01-14 at 16 40 09

Unable to upload files in editor.

@swissspidy
Copy link
Collaborator Author

jonny@macbook-pro web-stories-wp (try/wp-env ✗)]$ npm --version
6.14.11
[jonny@macbook-pro web-stories-wp (try/wp-env ✗)]$ nvm --version
0.37.2
[jonny@macbook-pro web-stories-wp (try/wp-env ✗)]$

And node --version?

@spacedmonkey
Copy link
Contributor

Node - v14.15.4

Comment on lines -194 to -196
"env:start": "./bin/local-env/start.sh",
"env:stop": "./bin/local-env/stop.sh",
"env:reset-site": "./bin/local-env/install-wordpress.sh --reset-site",
Copy link
Contributor

Choose a reason for hiding this comment

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

Any chance we could just make these aliases? Means I don't have to change what I try and no learning curve for current devs on the project.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah maybe we can keep them for some time.

@spacedmonkey
Copy link
Contributor

So the new setup doesn't create users and import media. Doesn't this break the e2e tests. How does this work if author isn't created?

@spacedmonkey

This comment has been minimized.

@swissspidy
Copy link
Collaborator Author

So the new setup doesn't create users and import media. Doesn't this break the e2e tests. How does this work if author isn't created?

It still does, but you need to run npm run test:prepare:e2e (see prepare-e2e-tests.sh).

Screenshot 2021-01-14 at 17 07 05

Some reason I seem to be running an old version of web stories plugin when I start it up. Not sure how this happened

That... is strange.

Have you perhaps used wp-env in the past? Try npm run wp-env destroy to start fresh, and then npm run wp-env start -- --update to be safe

Which site are you on? http://localhost:8898 is the local development environment, http://localhost:8899 is the test environment

@spacedmonkey
Copy link
Contributor

Screenshot from 2021-01-14 17-20-30

Unable to get the box up on my linux laptop.

@spacedmonkey
Copy link
Contributor

Both the issues I have am having on linux and mac os are related to nodegit.

@swissspidy
Copy link
Collaborator Author

Hmm maybe I accidentally dropped the hotfix for nodegit. I‘ll check

@spacedmonkey
Copy link
Contributor

Have you perhaps used wp-env in the past? Try npm run wp-env destroy to start fresh, and then npm run wp-env start -- --update to be safe

The issue was my git pull didn't work so I was using an old version of plugin. I deleted the branch and rechecked out. Seemed to fix the problem. It because they are force pushes in the history.

@spacedmonkey
Copy link
Contributor

Worth noting, I started from a completely fresh install on my linux laptop.

@swissspidy
Copy link
Collaborator Author

@spacedmonkey So the two main remaining issues I see here are:

Both are quite significant, and require upstream changes.

Thus, I feel like we should put this ticket back into the backlog and revisit later. Thoughts?

@swissspidy swissspidy added the Status: Blocked On hold for the time being label Jan 21, 2021
Base automatically changed from main to _main March 18, 2021 18:48
@swissspidy swissspidy closed this Mar 18, 2021
@swissspidy swissspidy deleted the branch main March 18, 2021 18:51
@swissspidy swissspidy reopened this Mar 18, 2021
@swissspidy swissspidy changed the base branch from _main to main March 18, 2021 19:03
@swissspidy swissspidy closed this Mar 19, 2021
@swissspidy swissspidy deleted the try/wp-env branch July 20, 2022 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Blocked On hold for the time being Type: Infrastructure Changes impacting testing infrastructure or build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use wp-env package for Docker environment
2 participants