chore: Migrate tests to RTL (6) #7063
Conversation
|
Preview: https://patternfly-react-pr-7063.surge.sh A11y report: https://patternfly-react-pr-7063-a11y.surge.sh |
jpuzz0
force-pushed
the
chore/rtl-migrate-bundle-6
branch
from
77e8749 to
66eb2c7
Compare
jpuzz0
force-pushed
the
chore/rtl-migrate-bundle-6
branch
from
66eb2c7 to
cbe1904
Compare
jpuzz0
force-pushed
the
chore/rtl-migrate-bundle-6
branch
from
cbe1904 to
ea14d7d
Compare
| progressAriaLabel?: string; | ||
| /** Associates the ProgressBar with it's label for accessibility purposes. Required when title not used */ | ||
| progressAriaLabelledBy?: string; | ||
| /** Unique identifier for Progress. Generated if not specified. */ |
There was a problem hiding this comment.
nit. lowercase for progress here.
It also looks like the id is not generated.
There was a problem hiding this comment.
It doesn't get generated in this file, but it does inside of Progress, where we pass progressId to the id prop and this the logic in that component:
I could rephrase and say something like Generated inside of Progress if not specified.?
In terms of the case of progress, I assumed it to be capitalized as it's referring to the component, just as the above comment on line 46 refers to ProgressBar in similar TitleCase.
There was a problem hiding this comment.
The above should probably be in sentence case too, "progress bar".
I think the comment is fine as is for the progressId.
| test('in the DISCONNECTED state', () => { | ||
| const { asFragment } = render( | ||
| <SerialConsole | ||
| onData={jest.fn()} |
There was a problem hiding this comment.
Given that the myIdPrefix is something that may be prepended in multiple places, I think we may want to keep it in these testss
There was a problem hiding this comment.
Checked the props and XTermProps which are extended for SerialConsole and neither allow for an id;
| extraParams: { onSelect } | ||
| }; | ||
| const returnedData = selectable('', { column, rowIndex: 0, rowData: { selected: true } } as IExtra); | ||
| expect(returnedData).toMatchObject({ className: 'pf-c-table__check' }); |
There was a problem hiding this comment.
here you did delete the aspect of the test that verified that the selecting event handlers are getting triggered on click. Should we either make a new test to verify that?
There was a problem hiding this comment.
I don't recall why I removed those. Probably wasn't clear what was trying to be tested as they just read "selected" and "main select". I can add some userEvents back in here to check for the same things.
| } | ||
| ]; | ||
|
|
||
| actionConfigs.forEach(testCellActions); |
There was a problem hiding this comment.
The actions are dynamically built in the table, so testing that they are built and firing the right events at the right times is probably something we should keep.
There was a problem hiding this comment.
None of these iterated tests in transformers made much sense to me, so I can take another look, and I recall trying to get these to work, but might be something I make a separate story in our backlog for to address later.
There was a problem hiding this comment.
that'd be fine if you end up doing that i think
| }); | ||
| }); | ||
|
|
||
| describe('SVGDefsSetter', () => { |
There was a problem hiding this comment.
Should we open an issue documenting that this test was removed?
There was a problem hiding this comment.
I'll take another look before doing so, but probably will end up doing that.
| @@ -0,0 +1,3 @@ | |||
| // Jest Snapshot v1, https://goo.gl/fbAQLP | |||
|
|
|||
| exports[`SVGDefs should get #addDef and #removeDef from context 1`] = `<DocumentFragment />`; | |||
There was a problem hiding this comment.
this is an empty snapshot? is it service a purpose?
There was a problem hiding this comment.
Managed to come up with something for this, SVGDefsSetter, and the one from transformers. Please see updates in latest commit.
There was a problem hiding this comment.
Long story short for the SVGDefs test; Verifying functions are called on mount and on unmount implicitly prove the context props are passed down appropriately.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
5 participants
What: Closes #7046