Tests: Replace PhantomJS with Karma/Chrome

[dmethvin]

As part of this I also had to pull out the coverage code, which already seemed to be broken anyway, since it also used PhantomJS.

[mgol]

Is this based on Core changes?

Two things:

1. The Travis config changes need to be migrated from Core as well.
2. Some tests are failing in Chrome Headless, they need to be fixed before landing this.

[dmethvin]

This is failing right now, it seems because it needs sudo set in the Travis config. @timmywil @Krinkle before just blindly doing that would you mind looking at what I've set up and see if there is anything else required? It runs locally. I think I probably need some additional incantations in the Travis file besides sudo.

[dmethvin]

ohai @mgol ! what tests are failing? Nothing was failing locally.

[mgol]

As for sudo, as the Core Travis file says, it's currently required or Karma can't access the browser. You also need to add the chrome add-on:

addons:
  chrome: stable

Have a look at https://github.com/jquery/jquery/blob/master/.travis.yml, it's not that complex.

[mgol]

Tests are fine, false alarm. This is because our npm scripts setup is not ideal. npm install && npm test (known also as npm it) should work without further actions and currently you need to first build manually.

Core's package.json is, again, a good indication in what we may want here as well:
https://github.com/jquery/jquery/blob/365284240429d442c3fbe9f505c7b297425bc3a3/package.json#L73

[mgol]

Oh, the Travis ticket linked from Core's Travis config (i.e. travis-ci/travis-ci#8836) is fixed now. Perhaps you just need to add the chrome add-on now?

[mgol]

...and if just adding the Chrome add-on doesn't work you may want to try disabling the Chrome sandbox, here's how:
karma-runner/karma-chrome-launcher#158 (comment)

If that helps, please configure it so that on a local machine it doesn't use this sandboxless Chrome, only on Travis.

[Krinkle]

Yeah. Chrome’s sandbox doesn’t work anymore inside Docker containers as of a few releases ago. For the moment either requires VM/sudo, or disabling the sandbox via Karma. Could be conditional inside Gruntfile based on a Travis env var.

[mgol]

Thanks, Timo. I've tried it and it works! See the Core PR: jquery/jquery#4011

@dmethvin you can do something like what I did in Core.

[dmethvin]

Thanks @mgol, the tests seem to be working now. Do the changes look good to you?

[Krinkle]

I'm not 100% sure, but it seems like we don't need the custom files. With the custom things for core removed, it looks like they're just the default now. Do the tests can pass without these files?

[dmethvin]

@Krinkle the standard file seems to run fine, but when i use the debug file I don't see any output on the screen. Also, on Windows although I can run the tests for debug I'm not sure how to exit the browser it launches in a way that Karma understands it was successful. If I close the browser it thinks Chrome crashed so it tries to relaunch 2 times and then hangs my command prompt.

[mgol]

We don't have such an npm script in Migrate. You might want to migrate that from Core if you want but for now it seems you can just remove the whole env block?

[dmethvin]

I've removed the custom files. It looks like I can still debug locally and the tests pass. When I'm debugging should I actually see the QUnit tests in the window? I don't, although I can still debug and see the results from the console which is enough I suppose.

[Krinkle]

@dmethvin Yeah, the HTML is all there, so it is using the regular "Web/HTML" reporter of QUnit, but karma-qunit doesn't load the css file by default and doesn't create the <div id="qunit"> by default.

However, it will do both of those things if you set client.qunit.showUI = true in the Gruntfile/karma settings. Per karma-qunit#readme (lib/index.js#L15-L16, and src/adapter.js#L27-L31).

Should be harmless to just enable always.

[dmethvin]

Thanks @Krinkle! That fixes the display issue. There is still a problem with Karma detecting that I've closed the debugging browser. Not sure how to deal with that, any ideas? That issue isn't blocking anything though. I really want to land this soon to finish some other PRs stuck behind it. I think @mgol is traveling now so if it looks good to you I'll land it.

Issue was resolved.