-
Notifications
You must be signed in to change notification settings - Fork 1
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
RTL migration: labels #607
RTL migration: labels #607
Conversation
@josiasds Hold off on the review for now; I just realized |
const wrapper = mount(<InputLabel name={name} label={false} />) | ||
expect(wrapper.find('label').exists()).toEqual(false) | ||
render(<InputLabel name={name} label={false} />) | ||
expect(screen.queryByText(formattedName)).not.toBeInTheDocument() |
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'm not sure this test works as intended. It still would pass if a label was rendered. It just tests the variable, which is never passed anyway.
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.
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 I just noticed that InputLabel
converts the text using convertNameToLabel
internally. So that's why you're testing against this variable.
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.
Testing against formattedName
didn't make sense to me because you never passed this value anyway. I see the connection now.
In other words, you could pass any label to the component (label="foo"
) and the test still wouldn't break.
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.
Yes that is true, if you passed in a different label the test would need to be updated to search for that label
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.
Anyway, since you're strictly testing for label={false}
in this test, I think we're fine.
* Set up React Testing Library * Migrate Spinner * convert button tests from enzyme to rtl * remove unused imports * remove unused import * Update jest and set test environment to jsdom * Bump version * update checkbox.test.js tests * checkbox.test.js and checkbox-group.test.js complete * update button.test.js to use userevent * update color-input.test.js to rtl * remove unused imports * update date input * complete color-input and date-input * complete dropdown-checkbox-group * complete hidden input * complete icon-input * Fix date input issues * RTL migration: controls (#602) * Migrate paginator * Add comment * Remove unused import * Migrate tab-bar * RTL migration: modal (#601) * Initial commit * Clean up logs * Add tests more default close interactions * RTL migration: indicators (#600) * Migrate loading container * Migrate flash message * Migrate flash message container * Upgdate assertion * Partial cleanup; Address code review * Remove unecessary test * Remove moment dependency from DateInput test * Update ColorPicker component * Update DropdownCheckboxGroup * Update HiddenInput * Update IconInput * Update Spinner component * Bump minor version * Address comments * Fix trigger on keys util * Migrate color-picker * RTL migration: tables (#603) * RTL migration: labels (#607) * partial label folder tests * Clean up error label * Fix input error tests * Remove unused attribute * Fix input label tests * Migrate labeled-field tests * Fixes --------- Co-authored-by: Alex Jin <alex@launchpadlab.com> * RTL migration: inputs (#604) * Migrate input * Migrate textarea * Remove unused import * Migrate switch * Remove unused import * Migrate range-input * Migrate select * Migrate masked input * Migrate replace empty string value hoc * Migrate blur dirty * Migrate radio group input * Migrate cloudinary uploader * Be more explicit * Use toHaveAttribute * Do not reference id directly * RTL migration: file inputs (#590) * Migrate FileInput to RTL * Migrate CloudinaryFileInput to RTL --------- Co-authored-by: Conor <conor@launchpadlab.com> * Remove enzyme * Add act back to file input * Avoid race conditions with act * Update lock * Add test for read helper * Mock server...better * Address uncovered line in wrap-display-name * Remove unused default * Add coverage for modal * Increase dropdown select coverage * Increase color picker coverage * Increase color-input coverage * Increase to-hex coverage * Increase paginator coverage * Increase masked input coverage * Increase date input coverage * Increase radio group coverage * Improve cloudinary-uploader coverage * Improve coverage for getEnvVar * Improve sortable table coverage * Improve tab-bar coverage * Update trigger on keys * Add comment * Replace act with waitFor --------- Co-authored-by: Alex Jin <alex@launchpadlab.com> Co-authored-by: Conor Hawes <conor@launchpadlab.com>
Migrates the label tests from Enzyme to RTL.
Creating a new PR to limit the broadcast of notifications.