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

[GB 15.8][E2E][Instagram Block] Use a different locator depending on the type of site for clicking the "Embed" button #77228

Merged
merged 1 commit into from
May 23, 2023
Merged

[GB 15.8][E2E][Instagram Block] Use a different locator depending on the type of site for clicking the "Embed" button #77228

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 12 additions & 1 deletion packages/calypso-e2e/src/lib/blocks/block-flows/instagram.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { envVariables } from '../../..';
import { BlockFlow, EditorContext, PublishedPostContext } from '.';

interface ConfigurationData {
Expand Down Expand Up @@ -34,7 +35,17 @@ export class InstagramBlockFlow implements BlockFlow {
await editorCanvas
.getByPlaceholder( 'Enter URL to embed here…' )
.fill( this.configurationData.embedUrl );
await editorCanvas.getByRole( 'button', { name: 'Embed' } ).click();

if ( envVariables.TEST_ON_ATOMIC ) {
await editorCanvas.getByRole( 'button', { name: 'Embed' } ).click();
} else {
// For some reason, the hierarchy/selector changes for simple sites. Probably
// because it's running from within the Gutenframe?
await editorCanvas
Copy link
Contributor Author

@fullofcaffeine fullofcaffeine May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm honestly not sure if it's related to the Gutenframe or some other modification that's simple-specific. I decided not to spend much time on it since this solution works; but it'd be nice if we could find a single locator that would work for both in the future, though not critical -- I think it's also OK to use a conditional around the site type if needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why await editorCanvas.getByRole( 'button', { name: 'Embed' } ).click() would not work for both cases here. Were you seeing a failure on Simple sites?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the new selector is not working, oddly.

locator.click: Error: strict mode violation: locator('body.block-editor-page').getByRole('button', { name: 'Embed' }) resolved to 2 elements:
1) <button tabindex="0" type="button" id="id-b5pcv7-1" aria…>…</button> aka getByRole('toolbar', { name: 'Block tools' }).getByRole('button', { name: 'Embed' })
    2) <button type="submit" class="components-button is-primar…>Embed</button> aka getByRole('document', { name: 'Block: Embed' }).getByRole('button', { name: 'Embed' })
=========================== logs ===========================
waiting for locator('body.block-editor-page').getByRole('button', { name: 'Embed' })
============================================================
    at InstagramBlockFlow.configure (/home/teamcity-0/buildAgent/work/c4a9d5b38c1dacde/packages/calypso-e2e/src/lib/blocks/block-flows/instagram.ts:40:64)
    at Object.<anonymous> (/home/teamcity-0/buildAgent/work/c4a9d5b38c1dacde/test/e2e/specs/blocks/shared/block-smoke-testing.ts:67:7)

Copy link
Member

@WunderBart WunderBart May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If this is failing for you locally only (CI doesn't complain), it might be because you're AutoProxxied. There is a bunch of extra A8c DOM that can break the test, so it's a good practice to have the proxxy disabled when running e2e tests. It can be frustrating devex-wise because the TC requires us to be proxxied (so you'll need to toggle the proxxy a lot at times), but I don't think that we should account for the extra DOM in our tests because it's not what the end users see.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this is failing for you locally only (CI doesn't complain), it might be because you're AutoProxxied. There is a bunch of extra A8c DOM that can break the test, so it's a good practice to have the proxxy disabled when running e2e tests. It can be frustrating devex-wise because the TC requires us to be proxxied (so you'll need to toggle the proxxy a lot at times), but I don't think that we should account for the extra DOM in our tests because it's not what the end users see.

Good note of caution. This was an issue that appeared on CI though, and it appears to be a permanent failure.

Copy link
Contributor Author

@fullofcaffeine fullofcaffeine May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Were you seeing a failure on Simple sites?

Yes, I was seeing this exact failure on simple sites; see build 10060993 of [Gutenberg simple E2E tests edge (desktop)] as an example. The next build - 10061174 - which included this changeset - passed.

Copy link
Contributor Author

@fullofcaffeine fullofcaffeine May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I decided not to spend much time on it since this solution works;

The solution failed in the production test site for AT, probably due to the theme? More details in p1684866514069289/1684346413.217409-slack-CDRRDATPS. @worldomonation simplified and fixed it in #77239.

.getByRole( 'document', { name: 'Block: Embed' } )
.getByRole( 'button', { name: 'Embed' } )
.click();
}

await editorCanvas.getByTitle( 'Embedded content from instagram.com' ).waitFor();
}
Expand Down
Loading