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

Avoid selecting insertion point indicator via class name in tests #28185

Closed
kevin940726 opened this issue Jan 14, 2021 · 1 comment
Closed

Avoid selecting insertion point indicator via class name in tests #28185

kevin940726 opened this issue Jan 14, 2021 · 1 comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Enhancement A suggestion for improvement.

Comments

@kevin940726
Copy link
Member

kevin940726 commented Jan 14, 2021

What problem does this address?

As seen in some tests:

// Should show the blue line indicator somewhere.
const indicator = await page.$(
'.block-editor-block-list__insertion-point-indicator'
);

The insertion-point-indicator currently is just a div with a special class name block-editor-block-list__insertion-point-indicator which is an implementation detail.

<div className="block-editor-block-list__insertion-point-indicator" />

As discussed in #27260, we should avoid testing implementation details in our tests. Instead, there's often a better alternative, let it be text, aria-label, or other accessible attributes.

What is your proposed solution?

Since the insertion-point-indicator is just a visual cue of where to insert a new block, I can't think of any good accessible attributes for it. I believe we should fallback to the last resort: test IDs. As popularized by RTL, querying by test IDs is a good solution for elements like the insertion-point-indicator.

We can simply add a test ID like to the div like:

-<div className="block-editor-block-list__insertion-point-indicator" />
+<div className="block-editor-block-list__insertion-point-indicator" data-testid="insertion-point-indicator" />

Then we can query it by getByTestId('insertion-point-indicator') or page.$('[data-testid="insertion-point-indicator"]')

@kevin940726 kevin940726 added [Type] Enhancement A suggestion for improvement. [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. labels Jan 14, 2021
@kevin940726 kevin940726 changed the title Avoid selecting insertion point indicator via class name Avoid selecting insertion point indicator via class name in tests Jan 14, 2021
@kevin940726
Copy link
Member Author

kevin940726 commented Sep 25, 2022

We can now target the indicator by testid (in Playwright: data-testid=block-list-insertion-point-indicator) since #43798. Some tests haven't been migrated yet though (inserting-blocks and editing-widgets)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

1 participant