testdouble / jasmine-rails

A Jasmine runner for rails projects that's got you covered in both the terminal and the browser
http://rubygems.org/gems/jasmine-rails
MIT License
378 stars 154 forks source link

Properly exit PhantomJS when running from command line #156

Closed JackWCollins closed 9 years ago

JackWCollins commented 9 years ago

I added some success and failure handling to quit out of PhantomJS once the specs are done running. This issue was manifesting itself if I ran rake spec:javascript from the command line. Note: I am on a Windows PC, but I don't think that would matter here.

This fixes #155.

searls commented 9 years ago

I'm still trying to figure out why you're experiencing this and not others

JackWCollins commented 9 years ago

That's fair... I spent a day and a half looking for a configuration issue on my machine because I didn't think it could be a bug - no one else has reported it. But after stepping through the entire code path between jasmine-rails, jasmine, and PhantomJS I couldn't find a place where jasmine-rails actually told PhantomJS to exit. As soon as I added the explicit exit calls then it worked as desired.

Do you recall where these exit commands are supposed to come from?

JackWCollins commented 9 years ago

I should add - running the jasmine specs in the browser works just fine and that is how we've been using it. The reason we need the command line interface to exit is so that we can use the tests on our continuous integration server.

searls commented 9 years ago

PhantomJS typically exits when the script is finished. I suspect the reason that phantomjs is not exiting is because you have either production code or test code that is leaving something on the event loop that isn't clearing (perhaps a setInterval or a hanging XHR?)?

If you go back and comment out or exclude all your JS code & tests, does this behavior persist? If you try a new empty project on your machine, does the same behavior persist?

JackWCollins commented 9 years ago

I think you're right... I found the specific spec that was hanging. We are using the lodash debounce to avoid making a function call with every key press on a text field that has a $watch. In the spec, it looks like we are trying to override the _.debounce with a setTimeout:

beforeEach inject ($compile, $rootScope) ->
    jasmine.clock().install()
    # override to use fake setTimeout, since lodash is loaded before the fake clock
    _.debounce = (fn, waitMillis) -> return () -> setTimeout fn, waitMillis 

Then in an example spec:

 describe 'typing in dates', ->
     it 'updates timeRangeFrom on scope when the date is valid', ->
       ctrl.textRangeFrom = @oct_15_text
       ctrl.textRangeTo = @oct_16_text
       ctrl.$digest()
       ctrl.tick(1000)
       expect(ctrl.timeRangeFrom.valueOf()).toEqual(@oct_15_value)

I'm not in contact with the code author, so I'm not exactly sure what the reasoning with the beforeEach block is here. I did find this good StackOverflow explanation of how various timeout methods interact with Jasmine.

I have a hunch that the issue here is that the setTimeout specified here is messing with the setTimeout in jasmine-runner.js. That seems like the piece of code that would normally exit PhantomJS:

setTimeout(function() {
    window.callPhantom({
       event: 'exit',
       exitCode: exitCode
    });
 }, 1);

I will try to investigate more and come to a definitive result.

searls commented 9 years ago

Ah, that certainly makes more sense. I'll close for now unless your issue is broader than we suspect.

JackWCollins commented 9 years ago

I wanted to add what I found here in case anyone else ever runs into this. I needed to do two things to fix the issue. First, the jasmine.clock() was not being uninstalled after use in an afterEach block. So, I needed to add this to the hanging specs:

afterEach ->
  jasmine.clock().uninstall()

The _.debounce was also giving me a little trouble, but for a different reason. The jQuery datetimepicker that we were using was choking when trying to render the calendar. This was causing the specs to fail. I'm not exactly sure why, but since that wasn't at all the unit under test for our specs, I spied on the datetimepicker and returned an empty div.

spyOn($.fn, 'datetimepicker').and.returnValue('<div></div>')

With those two fixes, the test suite now executes and exits properly.

searls commented 9 years ago

Hey Jack, thanks for the follow-up. The in-browser jasmine runner just doesn't handle async (like _.debounce) very well, especially when you don't wrap affected tests in either the 1.x or 2.x async APIs.