-
Notifications
You must be signed in to change notification settings - Fork 15
Attach a stable testid to default baseElement #44
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
base: main
Are you sure you want to change the base?
Attach a stable testid to default baseElement #44
Conversation
src/pure.tsx
Outdated
| // default to document.body instead of documentElement to avoid output of potentially-large | ||
| // head elements (such as JSS style blocks) in debug output | ||
| baseElement = document.body | ||
| document.body.dataset.testid = 'test-body' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently attached the testid only in cases where baseElement was not provided.
So if consumers provides a different baseElement it might fail again.
Not sure if the library should always attach it or not.
src/pure.tsx
Outdated
| // default to document.body instead of documentElement to avoid output of potentially-large | ||
| // head elements (such as JSS style blocks) in debug output | ||
| baseElement = document.body | ||
| document.body.dataset.testid = 'test-body' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The id needs to be generated to be unique, see
nanoidfor example - ported in VItest as https://github.com/vitest-dev/vitest/blob/8353defd6743925f363a7cf4d04f0297eaecc9af/packages/utils/src/nanoid.ts#L5 testidname can be redefined by the user viabrowser.locators.testIdAttribute, I think just usingidshould be fine if it's not defined already, since the locator generator will always prefer it instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weirdly enough the generator prefers the text content in my attempt
I tried to look at https://github.com/sheremet-va/ivya/blob/56a31c992baab353f82b46f3cadde9292eb3076a/src/selectorGenerator.ts#L112C10-L112C29
Looks like it requires a deeper dive.
Maybe its a bug in the Ivya code ?
Screen.Recording.2025-12-15.at.18.51.39.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can take a different approach and grab the testIdAttribute from the config and use it?
|
Pushed a change where ids are generated using Let me know what you think about this approach |
test/render.test.tsx
Outdated
| if (task.file.projectName === 'prod (chromium)') { | ||
| skip('Cannot mock nanoid in prod build') | ||
| } | ||
| vi.mock('@vitest/utils/helpers', { spy: true }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not how vi.mock works. It is always hoisted to the top of the file, this is a compiler hint, not a function call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh thanks for clarifying
Updated and moved this mock to the top.
| const stuff = document.createElement('div') | ||
| stuff.textContent = 'DOM content that might change' | ||
| document.body.appendChild(stuff) | ||
| setTimeout(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this code doesn't break the test, that seems weird. With your changes or without
| import type { JSX } from 'react/jsx-runtime' | ||
|
|
||
| export function ComponentThatChanges({ | ||
| timeout = 1500, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not artificially make our tests slower. Is there a better way to test this?
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