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

chore: add js test with mocha and chai #137

Closed
wants to merge 1 commit into from

Conversation

glennr
Copy link
Contributor

@glennr glennr commented Oct 30, 2015

This commit adds a JS unit testing framework with mocha and chai.
Sample unit tests are provided for Comment and CommentList.

Fixes #9

@justin808
Copy link
Member

Wow! Thanks @glennr!

@alexfedoseev @robwise, let's chat later today about how this compares to what we were planning for our internal project.

Thanks again @glennr!

@robwise JS tests will eventually go into the react_on_rails generator.

@glennr
Copy link
Contributor Author

glennr commented Oct 31, 2015

@justin808 you're welcome - I hope it's useful. Either way I'm super interested in knowing what test stack you guys end up going with on your internal project.

@glennr
Copy link
Contributor Author

glennr commented Oct 31, 2015

I found an issue on this branch:

npm ERR! peerinvalid The package react@0.14.0 does not satisfy its siblings' peerDependencies requirements!
npm ERR! peerinvalid Peer react-bootstrap@0.27.1 wants react@>=0.14.0
npm ERR! peerinvalid Peer react-addons-test-utils@0.14.1 wants react@^0.14.1
npm ERR! peerinvalid Peer uncontrollable@3.1.3 wants react@>=0.11.0
npm ERR! peerinvalid Peer react-overlays@0.5.0 wants react@>=0.14.0

I'll push a fix for this.

@justin808
Copy link
Member

@glennr I'm trying to run this, but it looks like a few more hours before we can update all.

cd client 
rm npm-shrinkwrap.json
npm-check-updates -u
npm install
npm prune
npm shrinkwrap

See #35.

  ~/shakacode/react-webpack-rails-tutorial/client (update-node-dependencies u=) ✗ npm install                                                                                                                                                                   ✹ [21:12:13]
- esprima-fb@15001.1001.0-dev-harmony-fb node_modules/defs/node_modules/esprima-fb
react-webpack-rails-tutorial@1.1.0 /Users/justin/shakacode/react-webpack-rails-tutorial/client
├─┬ UNMET PEER DEPENDENCY babel-core@6.0.14
│ ├── babel-code-frame@6.0.14
│ ├── babel-generator@6.0.14
│ ├── babel-helpers@6.0.14
│ ├── babel-messages@6.0.14
│ ├── babel-runtime@6.0.14
│ ├─┬ babel-template@6.0.14
│ │ └── babel-runtime@5.8.29
│ ├─┬ babel-traverse@6.0.14
│ │ └── babel-runtime@5.8.29
│ ├─┬ babel-types@6.0.14
│ │ └── babel-runtime@5.8.29
│ ├─┬ babylon@6.0.14
│ │ └── babel-runtime@5.8.29
│ └─┬ regenerator@0.8.35
│   ├── esprima-fb@15001.1.0-dev-harmony-fb
│   └─┬ recast@0.10.24
│     └── esprima-fb@15001.1.0-dev-harmony-fb
├─┬ eslint@1.8.0
│ └─┬ optionator@0.5.0
│   └── wordwrap@0.0.3
├─┬ jade@1.11.0
│ └─┬ uglify-js@2.4.24
│   └─┬ yargs@3.5.4
│     └── wordwrap@0.0.2
└─┬ react-bootstrap@0.27.3
  └── babel-runtime@5.8.29

npm WARN EPEERINVALID babel-loader@5.3.3 requires a peer of babel-core@^5.0.0 but none was installed.

@glennr
Copy link
Contributor Author

glennr commented Oct 31, 2015

@justin808 I indeed forgot to npm-shrinkwrap after adding those deps (I recalled something to that extent, but couldn't find it in the Readme - I found it on your blog entry this morning).

So there's a couple of issues on this branch now

  • incompatibility of react-addons-test-utils (try 0.14.0 not 0.14.1)
  • resolve babel incompatibility (after shrinkwrap)

I'm on a terribad 3g connection at the moment, so npm install has been a 'special' experience for me all morning. I'll check back in another few hours when npm install has hopefully completed :-)

@glennr
Copy link
Contributor Author

glennr commented Oct 31, 2015

I had to upgrade react to 0.14.1 to be compatible with react-addons-test-utils. All specs pass OK.

I think a lot of my issues were to do with my npm version, which I upgraded;

glennr@Django client[gr_mocha_tests*] % npm -v
2.14.7
glennr@Django client[gr_mocha_tests*] % npm install npm -g
/usr/local/bin/npm -> /usr/local/lib/node_modules/npm/bin/npm-cli.js
npm@3.3.10 /usr/local/lib/node_modules/npm
glennr@Django client[gr_mocha_tests*] % npm -v
3.3.10

@justin808 - WRT the babel incompatibility - I had to run 'npm prune --production' to remove all local dev deps before npm shrinkwrap would work. Is that right?

@glennr
Copy link
Contributor Author

glennr commented Oct 31, 2015

@justin808 - I've left the relevant unsquashed commits so its easier for you to review - but happy to squash them once you've given the +1

@@ -18,6 +18,7 @@
.env
node_modules
npm-debug.log
client/npm-debug.log*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have this in there regardless! Good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I just realized npm-debug.log* will suffice. All I wanted to do is ignore those timestamped npm-debug.log.201501...etc files

@justin808
Copy link
Member

CC: @alexfedoseev @glennr @jbhatab @robwise @mapreal19 @josiasds

This looks great!

I say we merge it!

@glennr Go ahead and rebase to one commit with a nice message.

@samnang
Copy link
Contributor

samnang commented Nov 2, 2015

This looks great! 👍

Thank @glennr

@alex35mil
Copy link
Member

And my 👍

Thanks @glennr!

@josiasds
Copy link
Member

josiasds commented Nov 2, 2015

@glennr Great! 👍

@glennr
Copy link
Contributor Author

glennr commented Nov 3, 2015

Awesome!

This commit adds a JS unit testing framework with mocha and chai.
Sample unit tests are provided for Comment and CommentList.

Also updates react to 0.14.1 to satisfy a test-utils dependency
@justin808
Copy link
Member

@glennr Can you please resolve the merge conflict and rebase again, and I'll merge.

Thanks again. Awesome job!

@justin808
Copy link
Member

@glennr if you can take care of this, that would be great. I want to leave you in as the author.

@justin808
Copy link
Member

@glennr I'm taking care of this now.

@justin808
Copy link
Member

See #151

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

Successfully merging this pull request may close these issues.

6 participants