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 3rd parties to configure the ARTIFACTS_PATH defined in the custom e2e test environment configured at jest-environment-puppeteer #34797

Closed
thomasplevy opened this issue Sep 13, 2021 · 5 comments
Labels
[Package] E2E Tests /packages/e2e-tests [Type] Help Request Help with setup, implementation, or "How do I?" questions.

Comments

@thomasplevy
Copy link
Contributor

What problem does this address?

I'm in the process of upgrading my testing suites (which use @wordpress/scripts for e2e testing) to stop using jasmine in favor of jest-cicrus for the test runner. I had implemented a screenshot reporter that takes screenshots when a spec fails and uploads them to an s3 bucket.

Since @wordpress/scripts v16.0.0 this functionality is redundant as of the introduction of the same functionality (but a bit better with the HTML included, thanks <3).

For my current process I store the screenshots in a custom directory (for my projects it's tmp/e2e-screenshots in the root of the project). I'd like to completely remove my custom reporter in favor of the one in the jest-puppeteer-environment/index.js, but in doing so I'll have to update several repos to .gitignore the artifacts/ dir and additionally rework our existing CI tooling to upload to s3 from a new directory. None of this is a huge deal but...

What is your proposed solution?

If the ARTIFACTS_PATH was configurable, say through an ENV var, I wouldn't have to update much and could, instead, just pass a new path in a bootstrap file (shared between my projects), more precisely, I'd propose:

const ARTIFACTS_PATH = path.join( root, 'artifacts' );

be converted to:

const ARTIFACTS_PATH = process.env.WP_ARTIFACTS_PATH || path.join( root, 'artifacts' );

I've tested this locally on my projects and it satisfies my request without breaking anything. If this seems like an acceptable addition I'd be happy to submit a PR.

Thanks!

@annezazu annezazu added [Package] E2E Tests /packages/e2e-tests [Type] Help Request Help with setup, implementation, or "How do I?" questions. labels Sep 24, 2021
@gziolo
Copy link
Member

gziolo commented Oct 4, 2021

@kevin940726 or @adamziel, can you help here?

@kevin940726
Copy link
Member

Sounds good to me. Happy to help review the PR :)

@adamziel
Copy link
Contributor

adamziel commented Oct 4, 2021

I second @kevin940726

@thomasplevy
Copy link
Contributor Author

Thanks all, I'll submit my PR later today!

@thomasplevy
Copy link
Contributor Author

Implemented via #35371

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] E2E Tests /packages/e2e-tests [Type] Help Request Help with setup, implementation, or "How do I?" questions.
Projects
None yet
Development

No branches or pull requests

5 participants