Skip to content
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

No tests? #73

Closed
binarykitchen opened this issue Feb 25, 2015 · 12 comments
Closed

No tests? #73

binarykitchen opened this issue Feb 25, 2015 · 12 comments

Comments

@binarykitchen
Copy link

That's a nice component but how come there are no tests?

@ryanzec
Copy link
Contributor

ryanzec commented Feb 26, 2015

That is the first thing that I noticed. A component as complex as this without tests is just asking for trouble.

@binarykitchen
Copy link
Author

Yep ... I am already having trouble running Jest tests with a Rect component I own which is including react-select within :(

@ryanzec
Copy link
Contributor

ryanzec commented Feb 27, 2015

I can actually understand some things not having unit tests depending on the unit test setup. I use Mocha/Chai/JSDom to unit test my code and I can't unit test any visual based stuff but this component definitely has a lot of none-visual stuff that can be unit tested.

@binarykitchen
Copy link
Author

You can unit-test visual based React stuff with Jest very well!
http://facebook.github.io/jest/docs/tutorial-react.html

And the visual based stuff you are referring to, are mostly called end to end tests (aka E2E). Just run a standalone selenium server in a gulp task, then run a middleware server and at last have a Jasmine test runner. It works well for my other projects.

All I am saying here is that having no tests is not good. I do not like the idea to ship code without enough test coverage, especially for enterprise apps we are developing here.

@JedWatson
Copy link
Owner

I would really appreciate help implementing tests for this component.

I have a set of manual processes that I run through to test the component before every release (based on the example configurations), it would certainly take less work to merge PRs and manage releases if our tests were automated.

@binarykitchen you mention having this setup in place for other projects - can you help get them set up here as well?

Also @ryanzec is correct, there are several things that are non-visual and could easily be unit tested.

@binarykitchen
Copy link
Author

@JedWatson Use the gulp-jest plugin for unit tests. Works like a charm.

@JedWatson
Copy link
Owner

@binarykitchen I probably wasn't clear - I'm managing a heap of open source projects at the moment between React stuff, KeystoneJS and TouchstoneJS as well as a having a full time job.

If someone else is able to contribute unit tests for this component I would really appreciate it, otherwise I'll get to them when I can :)

@binarykitchen
Copy link
Author

fair enough - it is just the fact that code without test is never good. always enforce yourself never to ship code without test ;)

@mattdell
Copy link
Contributor

I've added Jest framework into react-select. Pending PR. #107

@JedWatson
Copy link
Owner

Basic tests are in - more to come.

@binarykitchen
Copy link
Author

good, way to go

@dcousens
Copy link
Collaborator

dcousens commented May 7, 2015

Always could use improvement. This issue isn't really quantifiable beyond a test count being > 1, so I'm closing.
@JedWatson if a given coverage requirement is wanted, lets set that up as a new issue.

@dcousens dcousens closed this as completed May 7, 2015
bkoltai pushed a commit to bkoltai/react-select that referenced this issue Sep 20, 2017
Update to JedWatson/react-select v1.0.0-rc.4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants