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

AT E2E: fix selector for Instagram block in editor and published state. #77239

Merged
merged 3 commits into from
May 23, 2023

Conversation

worldomonation
Copy link
Contributor

@worldomonation worldomonation commented May 23, 2023

Fixes #77190.

Proposed Changes

This PR updates the selector used for the Instagram block in both editor and published state, to work with Simple and AT sites.

Key changes:

  • narrow the selector in the editor canvas.
  • narrow the selector in the published state.

Testing Instructions

Passes on both AT and Simple runs locally.

AT

➜  ~/workspace/wp-calypso-third git:(trunk) ✗ TEST_ON_ATOMIC=true yarn workspace wp-e2e-tests test -- test/e2e/specs/blocks/blocks__core-jetpack-extended.ts
 PASS  specs/blocks/blocks__core-jetpack-extended.ts (24.217 s)
  Blocks: Jetpack Extended Core Blocks
    ✓ Go to the new post page (13795 ms)
    Add and configure blocks in the editor
      ✓ Instagram: Add the block from the sidebar (3128 ms)
      ✓ Instagram: Configure the block (891 ms)
      ✓ Instagram: There are no block warnings or errors in the editor (3 ms)
      ✓ Twitter: Add the block from the sidebar (1029 ms)
      ✓ Twitter: Configure the block (387 ms)
      ✓ Twitter: There are no block warnings or errors in the editor (3 ms)
    Publishing the post
      ✓ Publish and visit post (1954 ms)
    Validating blocks in published post.
      ✓ Instagram: Expected content is in published post (13 ms)
      ✓ Twitter: Expected content is in published post (212 ms)

Test Suites: 1 passed, 1 total
Tests:       10 passed, 10 total
Snapshots:   0 total
Time:        24.422 s
Ran all test suites matching /test\/e2e\/specs\/blocks\/blocks__core-jetpack-extended.ts/i.

Simple

➜  ~/workspace/wp-calypso-third git:(trunk) ✗ TEST_ON_ATOMIC=false yarn workspace wp-e2e-tests test -- test/e2e/specs/blocks/blocks__core-jetpack-extended.ts
 PASS  specs/blocks/blocks__core-jetpack-extended.ts (20.582 s)
  Blocks: Jetpack Extended Core Blocks
    ✓ Go to the new post page (4927 ms)
    Add and configure blocks in the editor
      ✓ Instagram: Add the block from the sidebar (3549 ms)
      ✓ Instagram: Configure the block (459 ms)
      ✓ Instagram: There are no block warnings or errors in the editor (4 ms)
      ✓ Twitter: Add the block from the sidebar (1930 ms)
      ✓ Twitter: Configure the block (470 ms)
      ✓ Twitter: There are no block warnings or errors in the editor (4 ms)
    Publishing the post
      ✓ Publish and visit post (6072 ms)
    Validating blocks in published post.
      ✓ Instagram: Expected content is in published post (8 ms)
      ✓ Twitter: Expected content is in published post (259 ms)

Test Suites: 1 passed, 1 total
Tests:       10 passed, 10 total
Snapshots:   0 total
Time:        20.78 s, estimated 22 s
Ran all test suites matching /test\/e2e\/specs\/blocks\/blocks__core-jetpack-extended.ts/i.

Ensure the following build configurations are passing:

  • Gutenberg E2E Simple production (desktop)
  • Gutenberg E2E Atomic production (desktop)
  • Gutenberg E2E Simple edge (desktop)

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)?

- fix selector for both AT and Simple
- fix selector in validation to work with AT
@github-actions
Copy link

github-actions bot commented May 23, 2023

@worldomonation worldomonation marked this pull request as ready for review May 23, 2023 05:24
@worldomonation worldomonation requested a review from a team as a code owner May 23, 2023 05:24
@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 May 23, 2023
@worldomonation worldomonation self-assigned this May 23, 2023
@worldomonation worldomonation requested review from fullofcaffeine and WunderBart and removed request for a team May 23, 2023 05:24
Comment on lines +38 to +43
await editorCanvas
.getByRole( 'document', { name: 'Block: Embed' } )
.getByRole( 'button', {
name: 'Embed',
} )
.click();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've verified that this selector works for both AT and Simple, on Gutenberg Production and Edge.

@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.

Comment on lines 55 to 60
await context.page
.getByRole( 'figure' )
.filter( {
hasText: 'View this post on Instagram',
} )
.waitFor();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous selector was matching on two elements (somehow).

Copy link
Member

Choose a reason for hiding this comment

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

Possibly due to proxxied content?

Comment on lines 54 to 59
await context.page
.getByRole( 'figure' )
.filter( {
hasText: 'View this post on Instagram',
} )
.waitFor();
Copy link
Member

Choose a reason for hiding this comment

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

Would this work as well?

Suggested change
await context.page
.getByRole( 'figure' )
.filter( {
hasText: 'View this post on Instagram',
} )
.waitFor();
await context.page
.getByText( 'View this post on Instagram' )
.waitFor();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see why it wouldn't work - let me give it a try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to work locally with proxy on and off.

Triggering a Gutenberg build in CI to make sure it works there too.

Copy link
Member

@WunderBart WunderBart left a comment

Choose a reason for hiding this comment

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

🚢

@worldomonation worldomonation merged commit 8d4b5f0 into trunk May 23, 2023
@worldomonation worldomonation deleted the fix/instagram-take-3-77190 branch May 23, 2023 09:56
@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 May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Flaky E2E: Expected content is in published post / Configure the block (Instagram Block)
3 participants