E2E: use API calls in Widgets: Legacy spec to deactivate/remove widgets before the spec is run. #74479

worldomonation · 2023-03-15T18:06:17Z

Proposed Changes

This PR updates the Widgets: Legacy spec to use API calls in order to first deactivate all widgets.

Context: Gutenberg 15.3.1 release.
Thread: p1678383789296349/1678381006.233999-slack-CBTN58FTJ

Mobile viewport is not supported due to #64536.

Testing Instructions

Ensure the following build configurations are passing:

Gutenberg E2E (desktop)
Gutenberg E2E (mobile) (suite is skipped)

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-ajp-p2)?

- add API methods to perform actions on widgets. packages/calypso-e2e/src/types/rest-api-client.types.ts - add types

- update Widgets: Legacy spec.

github-actions · 2023-03-15T18:06:32Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-60251

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-60251&env=jetpack

matticbot · 2023-03-15T18:15:46Z

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.

- add JSDoc. - remove console.log

worldomonation · 2023-03-15T21:41:46Z

test/e2e/specs/appearance/widgets__legacy-visibility.ts

-skipDescribeIf( envVariables.TEST_ON_ATOMIC )(
-	DataHelper.createSuiteTitle( 'Widgets' ),
+// Mobile viewport is skipped due to https://github.com/Automattic/wp-calypso/issues/64536.
+skipDescribeIf( envVariables.TEST_ON_ATOMIC || envVariables.VIEWPORT_NAME === 'mobile' )(


I've moved the skip clause if the viewport size is mobile up to the top-level describe block.

It felt silly, as mobile viewport, to go through the setup for this spec only to not actually execute the all-important check.

worldomonation · 2023-03-15T21:42:17Z

test/e2e/specs/appearance/widgets__legacy-visibility.ts

-					await page.fill( 'input[placeholder="Search"]', 'Top Posts and Pages' );
-					await page.click( 'button.editor-block-list-item-legacy-widget\\/top-posts' );
-				} );
+		it( 'Insert a Legacy Widget', async function () {


The test hierarchy has been flattened here as the skip clause has been moved up to the top-level describe.

karenroldan

LGTM 🚀

karenroldan · 2023-03-16T09:49:02Z

test/e2e/specs/appearance/widgets__legacy-visibility.ts

-					await page.click( 'button.editor-block-list-item-legacy-widget\\/top-posts' );
-				} );
+		it( 'Insert a Legacy Widget', async function () {
+			await page.getByRole( 'button', { name: 'Add block' } ).click();


praise: I like how you changed this to use getByRole. Personally, I also prefer this approach and we give early feedback about the ARIA guidelines!

noahtallen · 2023-03-21T21:10:32Z

packages/calypso-e2e/src/rest-api-client.ts

+		if ( response.hasOwnProperty( 'error' ) && response.error === 'not_found' ) {
+			console.info( `Widget ${ widgetID } not found.` );
+			return;
+		} else if ( response === [] ) {


Hey @worldomonation, I noticed this while working on #74540 -- this probably doesn't work since JS will compare by reference, and a new empty array will always have a different reference.

What's the intended behavior here? (Empty array check?)

What's the intended behavior here? (Empty array check?)

Yes, that's correct - empty array check. I should have checked whether this was acceptable before putting it into production - looks like I should be checking with .length, correct?

PR opened: #74741

worldomonation added 2 commits March 15, 2023 11:01

packages/calypso-e2e/src/rest-api-client.ts

626e1a5

- add API methods to perform actions on widgets. packages/calypso-e2e/src/types/rest-api-client.types.ts - add types

test/e2e/specs/appearance/widgets__legacy-visibility.ts

00a8b92

- update Widgets: Legacy spec.

worldomonation self-assigned this Mar 15, 2023

worldomonation requested a review from fullofcaffeine March 15, 2023 18:06

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 15, 2023

worldomonation marked this pull request as ready for review March 15, 2023 19:04

worldomonation requested a review from a team as a code owner March 15, 2023 19:04

worldomonation changed the title ~~E2E: use API calls in Widgets: Legacy spec for more reliability.~~ E2E: use API calls in Widgets: Legacy spec to deactivate/remove widgets before the spec is run. Mar 15, 2023

packages/calypso-e2e/src/rest-api-client.ts

a5b2fda

- add JSDoc. - remove console.log

worldomonation commented Mar 15, 2023

View reviewed changes

karenroldan approved these changes Mar 16, 2023

View reviewed changes

worldomonation merged commit 7c11eda into trunk Mar 16, 2023

worldomonation deleted the update/e2e-widgets-legacy branch March 16, 2023 20:03

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 Mar 16, 2023

noahtallen reviewed Mar 21, 2023

View reviewed changes

worldomonation mentioned this pull request Mar 21, 2023

E2E Framework: fix empty array check in RestAPIClient. #74741

Merged

7 tasks

worldomonation mentioned this pull request May 10, 2023

Flaky E2E: Visibility options are shown for the Legacy Widget #72183

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

E2E: use API calls in Widgets: Legacy spec to deactivate/remove widgets before the spec is run. #74479

E2E: use API calls in Widgets: Legacy spec to deactivate/remove widgets before the spec is run. #74479

worldomonation commented Mar 15, 2023 •

edited

Loading

github-actions bot commented Mar 15, 2023 •

edited by matticbot

Loading

matticbot commented Mar 15, 2023

worldomonation Mar 15, 2023

worldomonation Mar 15, 2023

karenroldan left a comment

karenroldan Mar 16, 2023

noahtallen Mar 21, 2023 •

edited

Loading

worldomonation Mar 21, 2023

worldomonation Mar 21, 2023

E2E: use API calls in Widgets: Legacy spec to deactivate/remove widgets before the spec is run. #74479

E2E: use API calls in Widgets: Legacy spec to deactivate/remove widgets before the spec is run. #74479

Conversation

worldomonation commented Mar 15, 2023 • edited Loading

Proposed Changes

Testing Instructions

Pre-merge Checklist

github-actions bot commented Mar 15, 2023 • edited by matticbot Loading

matticbot commented Mar 15, 2023

worldomonation Mar 15, 2023

Choose a reason for hiding this comment

worldomonation Mar 15, 2023

Choose a reason for hiding this comment

karenroldan left a comment

Choose a reason for hiding this comment

karenroldan Mar 16, 2023

Choose a reason for hiding this comment

noahtallen Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

worldomonation Mar 21, 2023

Choose a reason for hiding this comment

worldomonation Mar 21, 2023

Choose a reason for hiding this comment

worldomonation commented Mar 15, 2023 •

edited

Loading

github-actions bot commented Mar 15, 2023 •

edited by matticbot

Loading

noahtallen Mar 21, 2023 •

edited

Loading