search

firehoseio / firehose

Build realtime Ruby web applications. Created by the fine folks at Poll Everywhere.
http://firehose.io/
MIT License
726 stars 72 forks source link

Add basic coffeescript tests with Evergreen #32

Closed brett-richardson closed 10 years ago

thoughtless commented 10 years ago

I understand that jQuery is needed for the tests to run, but I think it would be better if it lived in the spec directory. I believe the goal is for this to work with Zepto or jQuery. That probably means removing this line: https://github.com/brett-richardson/firehose/commit/8e3d8aeff2a196c79b0636ba44fc0327e621fb66#diff-99082413dfbf05673bd9e6aa0411a85fR3

I also see you have lib/assets/javascripts/vendor/json2.js. I agree that it belongs in a vendor directory. But you haven't removed lib/assets/javascripts/firehose/json2.js. We should only have one copy of that file.

One other minor point. Most of the developers with commit access to firehose are used to using Sinon.js for spies. I don't think that is a blocker though.

brett-richardson commented 10 years ago

I was thinking of simply mocking out the jQuery calls? We are only relying on the $.support and $.browser (deprecated) calls I think?

EDIT: Ok... long_poll accesses $.ajaxSettings.xhr() ... but I have mocked those out in the spec helper... so things run smoothly with or without jQuery now.

steel commented 10 years ago

Since $.browser is deprecated, we should replace it with a polyfill.

brett-richardson commented 10 years ago

@ieSupported:-> (document.documentMode || 10) > 9

I already did that with the above

brett-richardson commented 10 years ago

Travis pass! :laughing:

thoughtless commented 10 years ago

I'm liking this. @bradgessler any objections to merging this pull request?

bradgessler commented 10 years ago

None! Merge away!

On Friday, December 27, 2013 at 4:21 PM, Paul Cortens wrote:

I'm liking this. @bradgessler (https://github.com/bradgessler) any objections to merging this pull request?

— Reply to this email directly or view it on GitHub (https://github.com/polleverywhere/firehose/pull/32#issuecomment-31282810).