-
Notifications
You must be signed in to change notification settings - Fork 7
chore: Export TestSingleTabStopNavigationProvider to avoid a dependency on RTL #159
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
56fce5e to
5154281
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #159 +/- ##
==========================================
- Coverage 99.42% 99.33% -0.10%
==========================================
Files 35 36 +1
Lines 866 898 +32
Branches 234 236 +2
==========================================
+ Hits 861 892 +31
- Misses 5 6 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/internal/testing.ts
Outdated
| renderWithSingleTabStopNavigation, | ||
| TestSingleTabStopNavigationProvider, | ||
| getTestSingleTabStopNavigationProvider, | ||
| } from './single-tab-stop/__tests__/utils'; |
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 also has to change. We exclude tests directory from the typescript production build:
component-toolkit/tsconfig.json
Line 16 in 1bdc64f
| "exclude": ["**/__tests__/**", "**/__integ__/**"] |
Currently you see it working because of some unintended behaviors in the tooling, but this is not expected.
Cross-package code should be moved outside the __tests__ folder in something like test-utils if you would like to maintain a semantic name
| /** | ||
| * @deprecated - Use TestSingleTabStopNavigationProvider instead | ||
| */ | ||
| export function renderWithSingleTabStopNavigation( |
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.
Can you re-write this test-case to a non-deprecated version to prove that it works?
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 updated two tests to make sure both the provider and the helper util work as expected. Once components code is migrated, I will remove the renderWithSingleTabStopNavigation and migrate the rest of the tests.
5154281 to
9eb4acd
Compare
9eb4acd to
e64cd78
Compare
just-boris
left a comment
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.
Looks great, thanks
e64cd78 to
da18a55
Compare
|
@just-boris I updated the exported API a little so instead of |
The currently exported
renderWithSingleTabStopNavigationutil requires a dependency on the react testing library. To avoid having this dependency, we exportTestSingleTabStopNavigationProviderandsetTestSingleTabStopNavigationTargethelpers instead. Once components code is migrated, therenderWithSingleTabStopNavigationexport will be removed.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.