Improved test coverage #16
Conversation
- Got rid of JSXTransformer in test suite since it is not needed - Simplified firebaseRef validation (we ran into a Node bug with this old validation in GeoFire so we should change it here as well) - Added a bunch of tests to improve code coverage - Simplified existing tests
|
@mimming - I wanted to leave you with a fully tested library, so here it is. It would probably also be good to add some more complex tests (e.g. bind to multiple places as once, bind to the same place multiple times as an array and as an object, etc.). But this test suite should catch most issues. This also fixes a weird Node bug found in GeoFire. We should release 0.2.1 on Monday to get that fix out there. Fixes issue #4. |
| @@ -9,6 +9,8 @@ describe("ReactFireMixin Tests:", function() { | |||
|
|
|||
| describe("bindAsArray():", function() { | |||
| it("bindAsArray() throws errors given invalid Firebase refs", function() { | |||
| var expectedError = new Error("ReactFire: firebaseRef must be an instance of Firebase"); | |||
There was a problem hiding this comment.
binding the test to an error string seems kind of brittle. Is there any kind of error code or anything we can check for instead?
|
I'm still pretty green on this framework, but given that, everything in this commit looks pretty good. Do you trust me to merge it now, or should we wait until I get a better grip on the project? |
|
Cool. Let me try to address your one comment about error codes before merging this in. I should be able to look at it sometime soon. |
|
Back to you @mimming! |
|
Thanks for the change. It LGTM. Merging now. |
old validation in GeoFire so we should change it here as well)