Framework: Migrate "lib/accept" to single test runner #4503
Conversation
aduth
added
Framework
[Status] Needs Review
| confirmButtonText: React.PropTypes.string, | ||
| cancelButtonText: React.PropTypes.string, | ||
| translate: PropTypes.func, | ||
| message: PropTypes.node, |
There was a problem hiding this comment.
I didn't know there is node type :P Great to know 👍
There was a problem hiding this comment.
I didn't know there is node type :P Great to know 👍
As the original author of this component, neither did I at the time apparently 😆
|
Really great improvement. Tests works as expected and code looks nice. I would mark it as |
aduth
force-pushed
the
update/accept-single-test-runner
branch
from
389d615 to
5223896
Compare
|
@gziolo : Thanks for the feedback. I rebased and (force-)pushed some revisions to how I had tackled the i18n problem. Rather than auto-initializing in the case that a translate attempt is made before initialization, I created a separate helper in Let me know whether you think this is a better approach. |
| @@ -97,6 +97,7 @@ node_modules: package.json | node-version | |||
| test: build | |||
| @npm run test-client | |||
| @npm run test-server | |||
| @npm run test-tests | |||
gziolo
added
[Status] Ready to Merge
and removed
[Status] Needs Review
|
It looks great, I should run it in the browser. Give me couple of minutes :D |
gziolo
added
[Status] Needs Review
|
It also works as advertised
I think this is the best possible approach for all existing tests 👍 |
gziolo
added
[Status] Ready to Merge
and removed
[Status] Needs Review
aduth
force-pushed
the
update/accept-single-test-runner
branch
from
5223896 to
8893710
Compare
Related: #3942
This pull request seeks to migrate tests for
lib/acceptto the single test runner. It also includes revisions to thei18nmixin to better accommodate test contexts.Implementation notes:
translatestubbing has been swapped with the recently-addedlocalizehigher-order component. This alone was not sufficient to replace the existingtranslatestub, as thei18nmodule errors when trying to calltranslatebefore Jed has been initialized. To accommodate this, if a translate attempt is made before Jed is initialized, an empty Jed instance is created on-demand (23aed07). This should not affect existing browser usage asi18nis initialized during boot, but will simplify testing (avoiding need for explicit i18n initialization).Testing instructions:
Verify both that Mocha tests pass, and that no regressions occur in
acceptusage. An example ofacceptis the prompt which occurs when trying to delete media from your post editor media library./cc @rralian , @gziolo