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

Allow customization of the ARTIFACTS_PATH via WP_ARTIFACTS_PATH env var #35371

Merged
merged 8 commits into from
Oct 9, 2021

Conversation

thomasplevy
Copy link
Contributor

@thomasplevy thomasplevy commented Oct 5, 2021

Description

Per #34797

Allows 3rd-party customization of the ARTIFACTS_PATH used by @wordpress/scripts when running E2E tests by defining WP_ARTIFACTS_PATH as an environment variable.

If the env variable is empty, falls back to the default location.

How has this been tested?

I've manually patched my code into the @worpress/scripts package in my plugin's node_modules dir and run my e2e tests, forcing a failure and ran tests once without an env var set to ensure artifacts end up in the intended location and again after adding the new env var to ensure that artifacts end up in the custom location.

Edit: I've also tested this against this repo's npm run test-e2e both with the default usage (no ENV var) and with the ENV var set at runtime, eg: WP_ARTIFACTS_PATH=custom/artifacts npm run test-e2e

All these tests result in failed tests storing artifacts in the expected location

Screenshots

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • 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 (please manually search all *.native.js files for terms that need renaming or removal).

@thomasplevy thomasplevy marked this pull request as draft October 5, 2021 22:19
@thomasplevy thomasplevy marked this pull request as ready for review October 5, 2021 22:42
@talldan talldan added [Package] Scripts /packages/scripts [Type] Enhancement A suggestion for improvement. labels Oct 6, 2021
@talldan
Copy link
Contributor

talldan commented Oct 6, 2021

It would be good to update the changelog file for the scripts package, and maybe add some documentation along with this change.

Some details here on updating the changelog - https://github.com/WordPress/gutenberg/blob/HEAD/packages/README.md#maintaining-changelogs.

@thomasplevy
Copy link
Contributor Author

@talldan Of course!

I'm not sure if the location of the documentation makes sense. I didn't see anything specific in the README already about the artifacts where it would make sense to add information about the update in this PR so I added a new section.

Also, I don't see any other section with more than one <h4> heading, each has only a single <h4> for "Additional Information" so maybe this should belong under there? I'm not clear what the precedent is here and I'm happy to move it around if I've broken an established formula that's not apparent to me.

Also, of course, notes welcome on the language and wording I've chosen.

Thanks!

talldan
talldan previously approved these changes Oct 8, 2021
Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for updating the PR!

packages/scripts/README.md Outdated Show resolved Hide resolved
Copy link
Member

@kevin940726 kevin940726 left a comment

Choose a reason for hiding this comment

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

LGTM 👍. Thank you for making this PR ♥️

@kevin940726 kevin940726 merged commit ef26639 into WordPress:trunk Oct 9, 2021
@github-actions github-actions bot added this to the Gutenberg 11.8 milestone Oct 9, 2021
@thomasplevy
Copy link
Contributor Author

Awesome thanks @kevin940726

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Scripts /packages/scripts [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants