-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add initial set of tests #51
Conversation
Kris, great work on getting tests setup here! 🎉 About |
require( "cldr-data/main/en/currencies" ), | ||
require( "cldr-data/supplemental/currencyData" ), | ||
require( "cldr-data/supplemental/likelySubtags" ) | ||
); |
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 assume the specificity of CLDR content isn't subject to react-globalize tests goal, so I would simply replace these with:
Globalize.load(
require( "cldr-data" ).entireSupplemental(),
require( "cldr-data" ).entireMainFor( "en" )
);
Ideally, we could place this in a before
step for all tests and don't need to write this in every files. :)
I would be fine with that but maybe create an issue and do that separately since it would impact more than just tests |
const wrapper = shallow(<FormatMessage locale="en" path="task" variables={{count: 1}} />); | ||
expect(wrapper.text()).to.eql("You have one task remaining"); | ||
}); | ||
}); |
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.
We need tests using default message (no path
attribute).
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 wasn't meant to be a comprehensive test suite but instead a start and we can add as we go. That said, I will add that test as it will fail during development of the solution for #34 which will be helpful
Are there any options that we list in our documentation (i.e., README.md) that are not being tested? |
Ah! One is |
Definitely. Like I mentioned in a previous line comment, this was not meant to be a comprehensive set of tests on this first pass. My goal was to get the structure in place and some initial tests and folks can add tests as they have time. |
"eslint": "^1.10.3", | ||
"eslint-config-defaults": "^7.1.1", | ||
"eslint-plugin-react": "^3.11.2", | ||
"globalize": ">= 1.0.0 || 1.1.0-alpha - 1.1.x", |
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.
Let's stick with 1.1.x
here in devDependencies.
Great job so far! |
I believe I have addressed all comments except the following:
Otherwise, should be good to go and squashed the commits as well |
@kborchers, it LGTM 👍. |
Sets up Enzyme, Mocha, Chai tests run via Grunt. Also moves linting to a Grunt task.
Fixes #1