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

[fullofcaffeine]

Proposed Changes

See title. The DOM hierarchy seems different on simple sites. I didn't dig deeper into why - I just used the Playwright debugger to confirm the right selector for each case - but it could be due to simple sites running from the Gutenframe.

Testing Instructions

Tests should pass.

[github-actions]

Calypso Live (direct link)

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

Jetpack Cloud live (direct link)

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

[fullofcaffeine]

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.

[worldomonation]

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?

[worldomonation]

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)

[WunderBart]

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.

[worldomonation]

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.

[fullofcaffeine]

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.

[fullofcaffeine]

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.

[matticbot]

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.

[worldomonation]

@fullofcaffeine I know it might have been a time sensitive PR to get in, but would it be possible to wait for a review before merging to make sure we've done the right thing?

[fullofcaffeine]

@fullofcaffeine I know it might have been a time sensitive PR to get in, but would it be possible to wait for a review before merging to make sure we've done the right thing?

I understand your point, and on a high level, I agree with the sentiment, but let me give you my counter-arguments:

1. The process for upgrading the AT edge site is now automatic, and a new version can replace the current version in the edge site at any time. This can affect the progress on fixing builds as new code is pushed while we're testing. This shouldn't be a problem if builds pass on the first/second day after GB is released on core, but this release was already way behind schedule.

2. The release was behind schedule to the point of also affecting the simple release, too (though there were some manual workarounds, as I explained in the release threads for 15.8 (AT and simple) on Slack).

3. This build is only used by us (Calypso Platform team) now, so it's okay if the production build fails for a while. The code changed here shouldn't affect any other builds (though I might be wrong as I haven't worked on the Calypso E2E code extensively lately, if so, apologies).

4. The new AT release flow requires the builds to be green. We do have a workaround, but then we'd not rely on the new improved flow. Given this change was (probably) isolated (as I explained in point 4) above), I think trying to get a quick fix (and properly document it in this PR) was a reasonable tradeoff.

5. Getting a timely review would take at least another half day due to the timezone spread. Also, waiting on CI to finish the AT builds takes quite a while, and that adds up :/

That being said, it looks like this fix ended up failing. I could swear it worked well for me when testing on edge for AT and simple (I've posted links to the builds in the release threads), but maybe the problem was the difference in the GB version (production was probably still running 15.7.1 in the build you've shown). Next time I'll test across edge/production, too, I ended up focusing on edge * simple,atomic this time :/

EDIT: I also left some comments here + a conclusion/"micro-debrief": p1684866514069289/1684346413.217409-slack-CDRRDATPS.