Attach a stable testid to default baseElement#44
Attach a stable testid to default baseElement#44sheremet-va merged 18 commits intovitest-community:mainfrom
Conversation
|
Pushed a change where ids are generated using Let me know what you think about this approach |
|
@sheremet-va Sorry for the long delay. |
| return { | ||
| container, | ||
| baseElement, | ||
| locator: page.elementLocator(container), |
There was a problem hiding this comment.
Should we do a similar thing for the locator? Sorry, just noticed - It probably has the same issues
There was a problem hiding this comment.
Yea good call
After seeing youe message I tried to reprudoce this when using the locator and it does reprudoce.
Added a test-id to the container element when its not provided, similar to the baseElement
I'd like to make the snapshot of the first test ignore the test-id, not sure how
sheremet-va
left a comment
sheremet-va
left a comment
There was a problem hiding this comment.
This looks good to me, but I wonder if we should always apply a test id to this elements, not just when users don't pass anything. Just check the existence of the the attribute first
I guess if we’re only “ensuring” that a testid exists, then unless there’s a case where a user would intentionally want an element with no testid - which I can’t think of why atm - then this sounds good. Added a commit |
sheremet-va
left a comment
sheremet-va
left a comment
There was a problem hiding this comment.
Thank you for your hard work! 🫡 LGTM!
|
@sheremet-va test('should not override testid attribute if already set on baseElement', async () => {
const baseElement = document.createElement('div')
const screen = await render(<div>Render</div>, { container: baseElement })
})OR test('should not override testid attribute if already set on container', async () => {
const container = document.createElement('div')
const screen = await render(<div>Render</div>, { container })
})An exception is raised here: Kinda makes sense since the element is not in the document, but I wasn't aware of this. |
This is actually expected behavior (documented in testing-library and soon to be in vitest). But we should check it's in the DOM, we shouldn't add it ourselves |
got it! |
|
You know after released I thought that maybe it's better to have a more consistent id like `__vitest_${idx++}__`Since it appears in snapshots now. Maybe Vitest could also ignore data-attributes by default |
Port upstream fix from vitest-browser-react (vitest-community#42, vitest-community#44, vitest-community#45) to prevent flaky locators when baseElement/container lack a testid attribute. Co-Authored-By: Claude Opus 4.6 <[email protected]>
2 participants
When
baseElementis not provided andrender()falls back todocument.body, this PR sets adata-testidon the body to ensuregetElementLocatorSelectors(baseElement)generates a stable root selector.Without a stable attribute on the default
baseElement, the generated selector sometimes rely's on thedocument.bodytext content.This results in a flakey selector since sometimes the body includes some content that might change.
Adding the test id enforce the generated selector to be a stable testid-based selector.
Issue described here:
#42