-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Commands to run tests and storybook in StrictMode #3241
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
IMO better to have a single place to import all of out test utils, makes it easier to keep track
Build successful! 🎉 |
Build successful! 🎉 |
@@ -48,29 +44,13 @@ function ProviderUpdater(props) { | |||
return ( | |||
<Provider theme={theme} colorScheme={colorScheme} scale={scaleValue} locale={localeValue} toastPlacement={toastPositionValue}> | |||
<main> | |||
<div style={{position: 'absolute', paddingTop: '20px', paddingLeft: '20px', paddingRight: '20px'}}> |
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.
Clean up since we have a description tab in storybook now
@@ -13,7 +13,7 @@ | |||
import {ActionGroup, Item} from '../'; | |||
import {Provider} from '@react-spectrum/provider'; | |||
import React from 'react'; | |||
import {render} from '@testing-library/react'; | |||
import {render} from '@react-spectrum/test-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.
Re-exported all the testing library commands from test-utils
now that we are overriding render
, renderHooks
, and act
. Makes it easier IMO to just export from one place
let reactTestingLibrary = require('@testing-library/react'); | ||
export let renderHook = reactTestingLibrary.renderHook; | ||
export let actHook = reactTestingLibrary.act; | ||
if (!renderHook) { | ||
let rhtl = require('@testing-library/react-hooks'); | ||
renderHook = rhtl.renderHook; | ||
actHook = rhtl.act; |
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 stuff is moved to renderOverride.js
Build successful! 🎉 |
Build successful! 🎉 |
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.
LGTM, tested both locally and got expected runtime warnings and test failures for strict mode
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 good. I think eventually we'll want to have a setup where we can opt individual components into strict mode, and/or run both. That way we can verify there are no regressions before we finish updating all the components to support it. But this is a good start, good to merge IMO.
Build successful! 🎉 |
Closes
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: