-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(tests): Add mobile screenshot tests #28391
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
Conversation
9885d37 to
b0f3542
Compare
| * Search for a text broken up by multiple html elements | ||
| * e.g.: <div>Hello <span>world</span></div> | ||
| */ | ||
| export async function findByTextContent( |
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.
I think in the future we will have more utilities in this file
| ); | ||
| } | ||
|
|
||
| describe('StackTrace', function () { |
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 should not be part of this PR, but I could not resist, pls lmk if that is fine 😄
|
|
||
| return ( | ||
| <Role role={organization.attachmentsRole}> | ||
| <Role organization={organization} role={organization.attachmentsRole}> |
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 Role component is already wrapped with withOrganization but passing the org here was needed for the test to work. It was the simplest solution
d5d40f0 to
49f6573
Compare
|
|
||
| if (!tags.length && !hasContext && isShare) { | ||
| const screenshot = attachments.find( | ||
| ({name}) => name === 'screenshot.jpg' || name === 'screenshot.png' |
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 new condition is described in the draft PR: getsentry/develop#422
| textMatch: string | RegExp | ||
| ): Promise<HTMLElement> { | ||
| return screen.findByText((_, contentNode) => { | ||
| const hasText = (node: Element) => node.textContent === textMatch; |
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.
Would you be able to use node.innerText to search the element and the text of its children?
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.
It seems that innerText is not implemented in jsdom.. see here.
closes: INGEST-314
Added tests for the mobile screenshot feature. Related PRs:
#28309
#27350
#27274
#27228