Conversation
|
|
||
| // instead of default 'data-testid', use kibana's 'data-test-subj' | ||
| configure({ testIdAttribute: 'data-test-subj' }); | ||
| configure({ testIdAttribute: 'data-test-subj', asyncUtilTimeout: 4500 }); |
There was a problem hiding this comment.
before 4500 was a default. now it is 1000. Forcing 4500 back as some tests were failing
|
|
||
| describe('useFetcher', () => { | ||
| describe('when resolving after 500ms', () => { | ||
| let hook: ReturnType<typeof renderHook>; |
There was a problem hiding this comment.
Something around types of renderHook has changed that it is now more strict and doesn't allow use hook with this inferred type
| return <KibanaContextProvider services={services}>{children}</KibanaContextProvider>; | ||
| }; | ||
| return renderHook( | ||
| (props) => |
There was a problem hiding this comment.
Otherwise it says that props.options and others are missing. I didn't find a way how to nicely infer it from useMetricsExplorerData, so just copied types
| .find(`[data-test-subj="description-action"] [data-test-subj="property-actions-quote"]`) | ||
| .first() | ||
| .simulate('click'); | ||
| wrapper |
There was a problem hiding this comment.
Internal implementation of waitFor significantly changed between versions. Not sure why exactly, but I had to move this check inside waitFor to make test pass
|
|
||
| // Act | ||
| // Assert | ||
| expect(getByLabelText('Index pattern')).toBeInTheDocument(); |
There was a problem hiding this comment.
Here it threw error that Index pattern label is labelling span, which is incorrect.
I fixed the test, but this component should be reviewed from labels/a11y perspective
This was the error here:
TestingLibraryElementError: Found a label with the text of: Index pattern, however the element associated with this label (<span />) is non-labellable [https://html.spec.whatwg.org/multipage/forms.html#category-label]. If you really need to label a <span />, you can use aria-label or aria-labelledby instead.
There was a problem hiding this comment.
Thanks for flagging this up @Dosant . Yes we have a <span> inside an <EuiFormRow> on this page. Do you know what would be better from the a11y perspective? Should we replace the <span> with EuiText? This form row is displaying a non-editable piece of text (here 'gallery-*' is the span content):
| // check that all factories are displayed to pick | ||
| expect(screen.getAllByTestId(new RegExp(TEST_SUBJ_ACTION_FACTORY_ITEM))).toHaveLength(2); | ||
|
|
||
| expect(screen.getByText(/Go to URL/i)).toBeDisabled(); |
There was a problem hiding this comment.
There is button with a lot of nested content inside. It stopped returning a button, but returns a nested p instead.
| target: { value: URL }, | ||
| }); | ||
|
|
||
| [createHeading, createButton] = screen.getAllByText(/Create Drilldown/i); |
There was a problem hiding this comment.
createButton is a button with nested content inside. It stopped returning a button, but returns a nested p instead.
mistic
left a comment
There was a problem hiding this comment.
Changes on files under operations team code owners LGTM
|
Pinging @elastic/apm-ui (Team:apm) |
patrykkopycinski
left a comment
There was a problem hiding this comment.
😍 Thank you @Dosant (SIEM/Endpoint)
peteharverson
left a comment
There was a problem hiding this comment.
Transforms text edit LGTM.
Do you know what we should be using instead of the <span>?
|
|
||
| // Act | ||
| // Assert | ||
| expect(getByLabelText('Index pattern')).toBeInTheDocument(); |
There was a problem hiding this comment.
Thanks for flagging this up @Dosant . Yes we have a <span> inside an <EuiFormRow> on this page. Do you know what would be better from the a11y perspective? Should we replace the <span> with EuiText? This form row is displaying a non-editable piece of text (here 'gallery-*' is the span content):
💚 Build SucceededMetrics [docs]async chunks size
page load bundle size
History
To update your PR or re-run it, just comment with: |

Summary
Bumps
@testing-library/*libsNotable changes:
waitis deprecated.waitForandfindshould be used. docsuser-eventadd-on, which is better version of lower levelfireEventhttps://github.com/testing-library/user-eventNote:
@testing-library/react/pureinstead of@testing-library/reactandcleanUp()manually