Skip to content

6.2.1 #218

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

Merged
merged 18 commits into from
Aug 8, 2016
Merged

6.2.1 #218

merged 18 commits into from
Aug 8, 2016

Conversation

calvinmetcalf
Copy link
Contributor

what I like to see, only tests added

@mcollina
Copy link
Member

mcollina commented Jun 9, 2016

Tests failing :( some of them because of Error: ENOENT: no such file or directory, open '/home/travis/build/nodejs/readable-stream/test/fixtures/keys/ec-key.pem'

@calvinmetcalf
Copy link
Contributor Author

we don't actually need that file, it's for the c api

@lpinca
Copy link
Member

lpinca commented Jun 9, 2016

browser failures are probably caused by ngrok limits on free plan. Also there is bug in the ngrok module that prevents open tunnels from being closed, see defunctzombie/zuul#251 (comment).

In light of #217 it might make sense to switch back to localtunnel (it worked fine for me in the last few days.)

@calvinmetcalf
Copy link
Contributor Author

ok trying without ngrok

@mcollina
Copy link
Member

@calvinmetcalf why don't we ask ngrok if they could sponsor an account for CI to the Foundation? Or maybe we can buy one.

Another solution is to setup our own localtunnel server, I think we should try to avoid that as far as possible (but it's a solution).

@lpinca
Copy link
Member

lpinca commented Jun 10, 2016

Another option is to use karma + karma-sauce-launcher instead of zuul but the ppc issue will not be solved. I don't think there is a ppc binary for sauce-connect.

@lpinca
Copy link
Member

lpinca commented Jun 10, 2016

I think zuul can also run with sauce-connect and there is an addon for Travis, see https://docs.travis-ci.com/user/sauce-connect/#Setting-up-Sauce-Connect.

@calvinmetcalf
Copy link
Contributor Author

@lpinca thanks will try that

@calvinmetcalf
Copy link
Contributor Author

woot that works thanks @lpinca that being said there is a suble timing bug in one of the tests in 0.8 that just for fun, can be prevented by putting certain console.log statments in place

@calvinmetcalf
Copy link
Contributor Author

ok from what I can tell, in node 0.8 setTimeout(fn, 0) will sometimes comeback before process.nextTick

@calvinmetcalf
Copy link
Contributor Author

more specifically I think setTimeout(fn, 0) is the same as process.nextTick, making it setTimeout(fn, 1) fixes things

@calvinmetcalf
Copy link
Contributor Author

well 1 doesn't but 4 does

@mcollina
Copy link
Member

I think we should drop 0.8 asap and bump the major version.

@calvinmetcalf
Copy link
Contributor Author

naw I got 0.8 working

},
"scripts": {
"test": "tap test/parallel/*.js test/ours/*.js",
"browser": "npm run write-zuul && zuul -- test/browser.js",
"write-zuul": "printf \"ui: tape\ntunnel: ngrok\nbrowsers:\n - name: $BROWSER_NAME\n version: $BROWSER_VERSION\n\">.zuul.yml",
"write-zuul": "printf \"ui: tape\nsauce_connec: true\nbrowsers:\n - name: $BROWSER_NAME\n version: $BROWSER_VERSION\n\">.zuul.yml",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo here: sauce_connecsauce_connect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed thanks

@sonewman
Copy link
Contributor

hmm, there does seem to still be an issue :(

@lpinca
Copy link
Member

lpinca commented Jun 30, 2016

@sonewman if you are talking about CI, yes zuul is flaky as hell and sauce_connect makes it even more flaky :/

As @mcollina said, it would be great if we can get a ngrok account.

@calvinmetcalf
Copy link
Contributor Author

sign yeah I think your right

@mcollina
Copy link
Member

mcollina commented Jul 4, 2016

We should probably update this to 6.2.2 while we are at it #220.

@calvinmetcalf
Copy link
Contributor Author

ok added #220

@calvinmetcalf
Copy link
Contributor Author

though in this case it was because I a new es6 feature was introduced

@calvinmetcalf
Copy link
Contributor Author

bumped it, i don't think there is anything actually wrong with the pull, just the tests

@mcollina
Copy link
Member

@calvinmetcalf
Copy link
Contributor Author

yeah I take this back a browser is failing legitimately

@calvinmetcalf
Copy link
Contributor Author

testing manually to see if I can reproduce the android error

@calvinmetcalf
Copy link
Contributor Author

weird not able to reproduce the android error when running saucelabs locally

@lpinca
Copy link
Member

lpinca commented Jul 28, 2016

@calvinmetcalf I didn't have issues with localtunnel lately, maybe try to switch again? 😄

@calvinmetcalf
Copy link
Contributor Author

ok all browsers have had at least one green run so I'm calling this green enough

@calvinmetcalf calvinmetcalf merged commit ad2d515 into master Aug 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants