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

[14494] - Improve e2e testing docs and add cli args #14673

Closed
wants to merge 4 commits into from
Closed

[14494] - Improve e2e testing docs and add cli args #14673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
17ab0f2
No allignment gallery className fix.
nicolad Mar 24, 2019
2988861
Merge branch 'master' of https://github.com/WordPress/gutenberg
nicolad Mar 28, 2019
5cb0335
Updated readme and added local arg.
nicolad Mar 28, 2019
2f859f7
Revert "No allignment gallery className fix."
nicolad Mar 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/scripts/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ _Example:_
This is how you execute those scripts using the presented setup:

* `npm run test:e2e` - runs all unit tests.
* `npm run test-e2e interactive` - runs all unit tests interactively.
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it look as follows? /cc @draganescu

* `npm run test-e2e -- --puppeteer-interactive` - runs all unit tests interactively.
* `npm run test-e2e FILE_NAME -- --puppeteer-interactive ` - runs one test file interactively.
* `npm run test-e2e:watch -- --puppeteer-interactive` - runs all tests interactively and watch for changes.

In watch mode, you rather don't pass the name of the file as by default it runs only for modified files.

Copy link
Member Author

Choose a reason for hiding this comment

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

I made this change based on this PR's description: #14129

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, but I didn't update the PR's description with the changes in the comments. @gziolo is right, in the end it was developed as:

* `npm run test-e2e -- --puppeteer-interactive` - runs all unit tests interactively.
* `npm run test-e2e FILE_NAME -- --puppeteer-interactive ` - runs one test file interactively.
* `npm run test-e2e:watch -- --puppeteer-interactive` - runs all tests interactively and watch for changes.

* `npm run test-e2e interactive FILE_NAME` - runs one test file interactively.
* `npm run test-e2e:watch interactive` - runs all tests interactively and watch for changes.
* `npm run test-e2e:watch interactive FILE_NAME` - runs one test file interactively and watch for changes.
* `npm run test:e2e:help` - prints all available options to configure unit tests runner.

This script automatically detects the best config to start Puppeteer but sometimes you may need to specify custom options:
Expand Down
6 changes: 6 additions & 0 deletions packages/scripts/scripts/test-e2e.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,10 @@ if ( hasCliArg( '--puppeteer-interactive' ) ) {
process.env.PUPPETEER_SLOWMO = getCliArg( '--puppeteer-slowmo' ) || 80;
}

if ( hasCliArg( '--puppeteer-local' ) ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Hello @gziolo :)
I am not sure of 2 things here:

  1. whether local is the best arg name
  2. if checking arg with --puppeteer- as in example above is easy to reason about

Copy link
Member

Choose a reason for hiding this comment

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

Actually, the defaults are:

process.env.WP_BASE_URL = 'http://localhost:8889';
process.env.WP_USERNAME = 'admin';
process.env.WP_PASSWORD = 'password';

See: https://github.com/WordPress/gutenberg/blob/master/packages/e2e-test-utils/src/shared/config.js#L9

So there is no need to set them explicitly. We need the opposite, to have a way to override all those 3 values:

if ( hasCliArg( '--wordpress-host' ) ) {
    process.env.WP_BASE_URL = getCliArg( '--wordpress-host' );
}
if ( hasCliArg( '--wordpress-username' ) ) {
    process.env. WP_USERNAME = getCliArg( '--wordpress-username' );
}
if ( hasCliArg( '--wordpress-password' ) ) {
    process.env. WP_PASSWORD = getCliArg( '--wordpress-password' );
}

or even better something which uses look up table and loops over it.

process.env.WP_BASE_URL = 'http://localhost:8888';
process.env.WP_USERNAME = 'admin';
process.env.WP_PASSWORD = 'password';
}

jest.run( [ ...config, ...runInBand, ...getCliArgs( cleanUpPrefixes ) ] );