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.

Sign up for GitHub

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

Jump to bottom

wp-env: granular volume mappings #22256

Merged
merged 7 commits into from
May 12, 2020
Merged

wp-env: granular volume mappings #22256

merged 7 commits into from
May 12, 2020

Conversation

noahtallen
Copy link
Member

@noahtallen noahtallen commented May 10, 2020 •
edited
Loading

Description

Adds support for granular WordPress directory mappings. For example, you can do the following in .wp-env.json:

{
  "mappings": {
    "wp-content/plugins": "./path/to/lots-of-plugins"
  }
}

For the time being, I have only permitted local directory sources as values here. If folks want, though, it would be very easy to add git/zip source support to this as well.

How has this been tested?

Tested running wp-env start locally with these changes. I made sure that the mapped directory was mounted in WordPress, and that the plugins I expected to see also showed up in the plugins list. I also tested with themes.

Types of changes

New feature.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@noahtallen noahtallen added [Type] Enhancement A suggestion for improvement. [Package] Env /packages/env labels May 10, 2020
@noahtallen noahtallen self-assigned this May 10, 2020
@github-actions
Copy link

github-actions bot commented May 10, 2020 •
edited
Loading

Size Change: 0 B

Total Size: 827 kB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 4.08 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.62 kB 0 B
build/block-directory/style-rtl.css 764 B 0 B
build/block-directory/style.css 764 B 0 B
build/block-editor/index.js 104 kB 0 B
build/block-editor/style-rtl.css 10.6 kB 0 B
build/block-editor/style.css 10.6 kB 0 B
build/block-library/editor-rtl.css 7.12 kB 0 B
build/block-library/editor.css 7.12 kB 0 B
build/block-library/index.js 115 kB 0 B
build/block-library/style-rtl.css 7.38 kB 0 B
build/block-library/style.css 7.38 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 181 kB 0 B
build/components/style-rtl.css 17 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.43 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 4.42 kB 0 B
build/edit-navigation/style-rtl.css 618 B 0 B
build/edit-navigation/style.css 617 B 0 B
build/edit-post/index.js 28 kB 0 B
build/edit-post/style-rtl.css 12.2 kB 0 B
build/edit-post/style.css 12.2 kB 0 B
build/edit-site/index.js 12.1 kB 0 B
build/edit-site/style-rtl.css 5.22 kB 0 B
build/edit-site/style.css 5.22 kB 0 B
build/edit-widgets/index.js 8.37 kB 0 B
build/edit-widgets/style-rtl.css 4.69 kB 0 B
build/edit-widgets/style.css 4.69 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/index.js 44.3 kB 0 B
build/editor/style-rtl.css 5.07 kB 0 B
build/editor/style.css 5.08 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 734 B 0 B
build/format-library/index.js 7.63 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.14 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

@noahtallen noahtallen mentioned this pull request May 11, 2020
Closed
6 tasks
@noahtallen noahtallen force-pushed the add/env-granular-volume-mappings branch from 68af288 to 4e684bc Compare May 11, 2020 17:23
epiqueras
epiqueras reviewed May 11, 2020
View reviewed changes
.wp-env.json Outdated Show resolved Hide resolved
@epiqueras epiqueras mentioned this pull request May 11, 2020
Closed
6 tasks
@noahtallen noahtallen force-pushed the add/env-granular-volume-mappings branch from 8f82ef8 to c8a8482 Compare May 11, 2020 20:40
@noahtallen
Copy link
Member Author

interesting, I'm getting an error trying to run wp-env start that matches this issue: docker/compose#3633

@noahtallen noahtallen force-pushed the add/env-granular-volume-mappings branch from c8a8482 to 3682e30 Compare May 11, 2020 21:27
@noahtallen
Copy link
Member Author

Ah never mind that. It was because Docker wanted to install an update.

epiqueras
epiqueras approved these changes May 11, 2020
View reviewed changes
packages/env/README.md Outdated Show resolved Hide resolved
packages/env/README.md Outdated Show resolved Hide resolved
packages/env/test/config.js Outdated Show resolved Hide resolved
packages/env/package.json Outdated Show resolved Hide resolved
@noahtallen
Add mappings entry to config object
fcdab70
@noahtallen
Mount each "mapping" entry in Docker image
6fdf41b 
- Moved test plugins to .wp-env.json
@noahtallen
Add documentation
bbbb327
@noahtallen
Add tests for mapping
fc9529f 
- include new test file for build-docker-compose-config
@noahtallen
Support any source in mapping config
64222a3 
- Update changelog and readme to reference mu-plugins
- support git/zip sources
- add more examples to readme
- fix readme type
@noahtallen
Clarify readme
60e32ec
@noahtallen
Remove lodash, validate mapping keys
5a83a84
@noahtallen noahtallen force-pushed the add/env-granular-volume-mappings branch from 3682e30 to 5a83a84 Compare May 12, 2020 15:45
@noahtallen
Copy link
Member Author

Thanks for the comments, I've resolved each of those now :) I'll merge once I get a ✅ from travis

@noahtallen noahtallen merged commit 3616a16 into master May 12, 2020
@noahtallen noahtallen deleted the add/env-granular-volume-mappings branch May 12, 2020 16:08
@github-actions github-actions bot added this to the Gutenberg 8.2 milestone May 12, 2020
This was referenced May 21, 2020
Closed
Closed
@aduth
Copy link
Member

aduth commented May 22, 2020

Is there some way that mappings can differentiate between the test and non-test environment? For example, the current mu-plugins mapping for Gutenberg should arguably only apply for the test environment. The fact that they don't has caused some confusion in the past (e.g. #14245 (comment), #18761 (comment)).

I could possibly see an argument that the mappings should be kept simple and apply universally, though it's the same sort of argument I think would justify that "test" and "non-test" sites should just be multiple environments, not a first-class distinction.

I'm also open to alternative approaches. Curious if there are any thoughts on this.

  • Avoid using mu-plugins for end-to-end tests and just use regular plugins that are activated in the test environment. The primary downside here is that this causes some delay in test execution time.
  • Somehow detect environment from within the implementation of the mu-plugin as recognizing to be the test or non-test environment. Possibly via the availability of some environment variable exposed from wp-env ?

@aduth aduth mentioned this pull request May 22, 2020
Closed
@noahtallen
Copy link
Member Author

noahtallen commented May 22, 2020 •
edited
Loading

For example, the current mu-plugins mapping for Gutenberg should arguably only apply for the test environment.

Yeah, this has been an issue since wp-env was created. Before this PR, it was hidden in the code.

Is there some way that mappings can differentiate between the test and non-test environment?

Not yet. we started talking about this here: #21229 (comment)

I have a WIP branch which allows for different configurations in each environment, which would also solve #22514. I haven't made a lot of progress on it lately, but I can bring it back up since it comes up a lot.

@noahtallen
Copy link
Member Author

@aduth WIP pull request here if you'd like to follow along: #22568

sirreal added a commit that referenced this pull request Jun 1, 2020
@sirreal
Fix undefined Source type lint/type error
60334bf 
The following PRs were fine in isolation, but when merged resulted in
the introduction of an undefined `Source` type in the `env` package:

#22256
#20522

Rename `Source` introduced in #22256 to `WPSource` according to changes
in #20522.
@sirreal sirreal mentioned this pull request Jun 1, 2020
Merged
sirreal added a commit that referenced this pull request Jun 1, 2020
@sirreal
Env: Fix undefined Source type lint/type error (#22785)
7e855e3 
The following PRs were fine in isolation, but when merged resulted in
the introduction of an undefined `Source` type in the `env` package:

#22256
#20522

Rename `Source` introduced in #22256 to `WPSource` according to changes
in #20522.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@epiqueras epiqueras epiqueras approved these changes

@noisysocks noisysocks Awaiting requested review from noisysocks

Assignees

@noahtallen noahtallen

Labels
[Package] Env /packages/env [Type] Enhancement A suggestion for improvement.
Projects
None yet
Milestone
Gutenberg 8.2
Development

Successfully merging this pull request may close these issues.
3 participants
@noahtallen @aduth @epiqueras