-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Framework: Remove i18n mixin injections from tests #18471
Conversation
cccd62e
to
82d4b3f
Compare
82d4b3f
to
2957c58
Compare
721b008
to
85b8e57
Compare
import assert from 'assert'; | ||
import sinon from 'sinon'; | ||
import { identity } from 'lodash'; | ||
|
||
/** | ||
* External dependencies |
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.
Lies!
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.
Good spot. 2c2f670
I don't really know the old paradigm of mixin injection for tests. You are deleting setup code and tests still pass...so seems good to me! |
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.
Awesome cleanup, 🚢 it!
@samauri I think the reason is simply that
|
Remove i18n mixin injections from tests (and some obsolete
matches-selector
stubs I found while working on this). This PR really only touches test files -- the corresponding units have been usinglocalize
already, only the tests hadn't been updated accordingly yet.Rationale: We're trying to get rid of mixin auto-injection for the React 16 upgrade, since this relies on methods (
react/lib
) that aren't part of the public interface.To test: Verify that unit tests pass.
Follow-up PR that tackles
client/auth/test/login.jsx
which also requires changes to the tested unit.