n-riesco / ijavascript

IJavascript is a javascript kernel for the Jupyter notebook
Other
2.18k stars 187 forks source link

Run tests on Travis CI #99

Closed lgeiger closed 7 years ago

lgeiger commented 7 years ago

Currently the tests fail on CI and probably show the failure described in https://github.com/nteract/hydrogen/issues/591.

Here is a link to the CI build: https://travis-ci.org/lgeiger/ijavascript/builds/213530243

@n-riesco It would be great if you could enable Travis for this repository so tests are run on every commit and PR.

n-riesco commented 7 years ago

@rgbkrk @lgeiger I'd like to solve the building problems before merging this PR.

Any ideas why the build fails in Travis for node v0.10 and v0.12 and v4?

lgeiger commented 7 years ago

Any ideas why the build fails in Travis for node v0.10 and v0.12 and v4?

Could be caused by an old version of npm which may had different behavior in locating modules.

n-riesco commented 7 years ago

@lgeiger I guess that because of this line I need to declare jmp as a dev-dependence.

lgeiger commented 7 years ago

I guess that because of this line I need to declare jmp as a dev-dependence.

That could fix it. Since npm <= 2 had a different way of handling the install location of indirect dependencies.

n-riesco commented 7 years ago

@lgeiger Would you mind to update your PR with jmp as a dev-dependence? And check whether it builds on Travis?

lgeiger commented 7 years ago

It fixes the build issue: https://travis-ci.org/lgeiger/ijavascript/builds/213595444

It would be cool if you could enable Travis on this repo so we can see the build status in the PR directly.

n-riesco commented 7 years ago

If I merge now, what would happen to other PRs? Won't they fail because this PR doesn't setup Travis properly?

lgeiger commented 7 years ago

You can tell Travis to only build if .travis.yml is present:

bildschirmfoto 2017-03-21 um 22 47 36
n-riesco commented 7 years ago

The failure in v0.12 is expected. I used to workaround this issue. I guess we could bring back the workaround.

rgbkrk commented 7 years ago

I personally suggest not supporting v0.12, as it was end-of-life back on 2016-12-31. If you only have so much time for maintenance work, reduce the burden.

LTS Schedule

lgeiger commented 7 years ago

Sure, I remove it!

n-riesco commented 7 years ago

Please, keep v0.10 (this is the version that comes with Ubuntu 14.04LTS).

I've updated the tests to fix the failure in replies correctly to inspect_request: util.inspect.

Please, rebase to master.

lgeiger commented 7 years ago

Added 0.10 and rebased.

Though AFAIK Ubuntu 16.04 is the current LTS release.

n-riesco commented 7 years ago

Ubuntu LTS releases are supported for 5 years!

Also, IJavascript was developed in Ubuntu 14.04LTS and it still runs on it.

lgeiger commented 7 years ago

Ubuntu LTS releases are supported for 5 years!

👍

My main intention for this PR is that it shows random failures which may cause: https://github.com/nteract/hydrogen/issues/591 and to give a possible hint on how to reproduce.

For example: https://travis-ci.org/n-riesco/ijavascript/jobs/213628063

Therefor I don't really expect this PR to fully go green 💚

n-riesco commented 7 years ago

The first test to fail is replies correctly to execute_request: 'Hello, World!' (which is the first test to send messages on the IOSocket).

In some runs only the test replies correctly to execute_request: 'Hello, World!' fails. In the others, all the tests using IOSocket fail.

I have come across a similar issue with node-zmq when sending messages in a rapid succession. Unfortunately, I never got a reliable repro. I will have to try harder :(

n-riesco commented 7 years ago

Note to myself: since we are using zeromq (which uses zmq v4), try to enable monitor and do not send messages until the sockets have accepted a connection (ZMQ_EVENT_ACCEPTED).

lgeiger commented 7 years ago

@n-riesco Thanks for taking a look.

n-riesco commented 7 years ago

http://zguide.zeromq.org/page:all#Missing-Message-Problem-Solver

"slow joiner"