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

Add some Block Editor Tracking E2E tests #53733

Merged
merged 3 commits into from
Jun 21, 2021
Merged

Add some Block Editor Tracking E2E tests #53733

merged 3 commits into from
Jun 21, 2021

Conversation

david-szabo97
Copy link
Contributor

@david-szabo97 david-szabo97 commented Jun 16, 2021 •
edited
Loading

Changes proposed in this Pull Request

Testing instructions

cd test/e2e

# Page editor
./node_modules/.bin/mocha specs/specs-wpcom/wp-calypso-gutenberg-page-editor-tracking-spec.js
# Post Editor
./node_modules/.bin/mocha specs/specs-wpcom/wp-calypso-gutenberg-post-editor-tracking-spec.js
# Site Editor
./node_modules/.bin/mocha specs/specs-wpcom/wp-calypso-gutenberg-site-editor-tracking-spec.js

Related to #53410

@matticbot
Copy link
Contributor

matticbot commented Jun 16, 2021 •
edited
Loading

Link to Calypso live: https://calypso.live?image=registry.a8c.com/calypso/app:build-8496
Link to Jetpack Cloud live: https://calypso.live?image=registry.a8c.com/calypso/app:build-8496&env=jetpack

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@david-szabo97 david-szabo97 self-assigned this Jun 16, 2021
@david-szabo97 david-szabo97 marked this pull request as ready for review June 16, 2021 11:31
@david-szabo97 david-szabo97 requested review from scinos, WunderBart and a team as code owners June 16, 2021 11:31
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 16, 2021
Addison-Stavlo
Addison-Stavlo reviewed Jun 18, 2021
View reviewed changes
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

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

This is looking pretty good to me. Just a minor Q about creating removeBlock via selectors and timeouts vs. using the block editor store.

test/e2e/lib/components/site-editor-component.js Show resolved Hide resolved
test/e2e/lib/gutenberg/tracking/utils.js Show resolved Hide resolved
@Addison-Stavlo Addison-Stavlo mentioned this pull request Jun 18, 2021
Closed
24 tasks
@david-szabo97 david-szabo97 force-pushed the add/tracking-e2e-tests-1 branch from 83f5e35 to 2959724 Compare June 18, 2021 16:07
@david-szabo97 david-szabo97 force-pushed the add/tracking-e2e-tests-1 branch from 2959724 to 20f51cc Compare June 21, 2021 09:53
@david-szabo97 david-szabo97 merged commit 65dddcf into trunk Jun 21, 2021
@david-szabo97 david-szabo97 deleted the add/tracking-e2e-tests-1 branch June 21, 2021 10:15
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 21, 2021
WunderBart
WunderBart reviewed Jun 22, 2021
View reviewed changes
test/e2e/lib/components/site-editor-component.js
By.css( '.components-menu-group' ),
this.explicitWaitMS / 5
);
await this.driver.sleep( 1000 );
Copy link
Member

Choose a reason for hiding this comment

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

Why the extra sleep here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a copy-paste from GutenbergEditorComponent#removeBlock, but adapted to the site editor. The sleep is not documented there either, so I'm not quite sure why it's required.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, OK. Anyway, a good approach is to verify if those mysterious sleeps are actually necessary whenever bumped into one, especially when copypasting from somewhere else. This way we're not adding any unnecessary time to our E2Es.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WunderBart WunderBart WunderBart left review comments

@Addison-Stavlo Addison-Stavlo Addison-Stavlo approved these changes

@scinos scinos Awaiting requested review from scinos

@Copons Copons Awaiting requested review from Copons

Assignees

@david-szabo97 david-szabo97

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@david-szabo97 @matticbot @WunderBart @Addison-Stavlo