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

use @mocha/karma-sauce-launcher #2861

Closed
wants to merge 1 commit into from
Closed

Conversation

boneskull
Copy link
Member

after seeing some strange recent false positive builds and a history of flake, I forked karma-sauce-launcher which seems unmaintained; hopefully we'll have better results.

cc @Munter

@coveralls
Copy link

Coverage Status

Coverage remained the same at 88.288% when pulling 516f868 on new-sauce-launcher into 2928b38 on master.

@boneskull
Copy link
Member Author

apparently we need an Object.keys() polyfill now due to fuzzy version ranges. having trouble tracking it down. another vote for shrinkwrapping.

@boneskull
Copy link
Member Author

the good news is that the build has sped up and we are gaining PR access to SauceLabs and it looks like less flake.

@boneskull boneskull added the semver-patch implementation requires increase of "patch" version number; "bug fixes" label Jun 15, 2017
@boneskull
Copy link
Member Author

This needs #2868, then I can rebase onto it. This should not land in v3.5.0.

- straighten some spaghetti in `karma.conf.js`
- used increased concurrency; this *should* improve CI speed
- allow Travis-CI to open the SauceLabs tunnel and reuse it
- allow ES2015+ in config files
- make AWS region and bucket public
- make "http" tests more robust via `get-port` package
- conditionally run sauce connect
- allow PRs to execute sauce tests
@coveralls
Copy link

coveralls commented Jun 16, 2017

Coverage Status

Coverage remained the same at 88.395% when pulling c8cb3fd on new-sauce-launcher into 50fc47d on master.

@ScottFreeCode
Copy link
Contributor

So, I haven't looked at the changes in the fork -- although I presume they include a lot of needed fixes -- but one thing I'm wondering about is the mechanism by which they supersede Travis's JWT solution for PR builds. Can that be briefly but substantially described?

@ScottFreeCode
Copy link
Contributor

ScottFreeCode commented Jun 17, 2017

Unless I misunderstood and this PR is adding the JWT configuration, superseding prior attempts to set it up but not superseding the use of JWT as the solution? Because if that's the case then I have no major concerns about getting this done.

Well, we're going to have to be careful merging together the karma.conf.js changes from here and from my browser coverage PR, but I'm not sure there's much that can be done to alleviate that.

@ScottFreeCode
Copy link
Contributor

The PhantomJS failure (which I believe happens currently even without this PR) is due to the addition of a bind call in readable-stream version 2.3.0 released on June 19th.

The Object.keys use, which is triggerring failure in some of the IE versions, is in safe-buffer... however, it's in even relatively old versions of safe-buffer, and there doesn't appear to have been a change in safe-buffer dependency... so I am unsure what changed in how our dependencies use safe-buffer to cause it to be run in the browser when it wasn't before, especially since Browserify was not updated recently.

But maybe both are due to the readable-stream update?

@ScottFreeCode
Copy link
Contributor

#2898 should fix Travis, #2897 should fix AppVeyor.

@ScottFreeCode
Copy link
Contributor

Something I want to point out:

When I first saw this PR, I wondered what the rationale was for changing from a Travis job per each browser to just one Travis job for all the browsers.

Well, tonight I looked at a PR from a fork and noticed that Travis had passed all the jobs. "Wow!" I thought, "Did the readable-stream issue affecting Internet Explorer 7 and 8 get fixed?"

It isn't fixed; for pulls from forks, without the JWT feature to run Sauce, Travis is currently running Phantom on all those "Internet Explorer" and "Firefox" and "Chrome" jobs.

I approve of just about anything that fixes that!! 😆

@ScottFreeCode ScottFreeCode added area: repository tooling concerning ease of contribution qa labels Sep 3, 2017
@ScottFreeCode
Copy link
Contributor

The PRs that fix the Travis and AppVeyor problems both got merged, so we should be able to rebase this and then properly review it.

@boneskull boneskull closed this Sep 28, 2017
@boneskull boneskull deleted the new-sauce-launcher branch September 28, 2017 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: repository tooling concerning ease of contribution semver-patch implementation requires increase of "patch" version number; "bug fixes"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants