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

Added very very basic test #26

Merged
merged 5 commits into from
Jan 9, 2016
Merged

Added very very basic test #26

merged 5 commits into from
Jan 9, 2016

Conversation

sglyon
Copy link
Contributor

@sglyon sglyon commented Dec 21, 2015

Related to #25

@MikeInnes
Copy link
Contributor

Good stuff. Do you know if Electron will run on Travis?

@sglyon
Copy link
Contributor Author

sglyon commented Dec 22, 2015

Hmm, I don't think it will. I just tried in a docker image (as I think that shell, only VM is pretty close to travis) and it didn't work.

Oh well... Do you have any other ideas for how to get around that?

@EricForgy
Copy link

Hi @spencerlyon2

What was the error? It didn't like to popup a window? I wonder if setting an option

show: false

i.e.

julia> w = Window(Blink.@d(:show => false))

would work? See http://electron.atom.io/docs/v0.35.0/api/browser-window/

@EricForgy
Copy link

Hi @spencerlyon2 and @one-more-minute

I notice in your test you have a "sleep(5.0)". If my PR #39 is merged, you will no longer need this :)

@sglyon
Copy link
Contributor Author

sglyon commented Dec 28, 2015

@EricForgy I'm not sure what doesn't work here.

I'll give the :show => false idea a try -- good idea.

@sglyon
Copy link
Contributor Author

sglyon commented Dec 28, 2015

Didn't work:

julia> w = Window(Blink.@d(:show => false));
ERROR: connect: connection refused (ECONNREFUSED)
 in yieldto at ./task.jl:71
 in wait at ./task.jl:371
 in wait at ./task.jl:286
 in stream_wait at ./stream.jl:60
 in wait_connected at ./stream.jl:331
 in connect at socket.jl:669
 in try_connect at /root/.julia/v0.4/Blink/src/AtomShell/process.jl:49
 in init at /root/.julia/v0.4/Blink/src/AtomShell/process.jl:62
 in shell at /root/.julia/v0.4/Blink/src/AtomShell/process.jl:102
 in call at /root/.julia/v0.4/Blink/src/AtomShell/window.jl:37

This might be some strange thing running in a docker vm though, not sure. If I find some time I'll try to debug

@EricForgy
Copy link

The error message is actually encouraging :)

I've never done anything with Travis. How difficult would it be to just try it instead of debugging Docker? :)

@sglyon
Copy link
Contributor Author

sglyon commented Dec 28, 2015

I've updated the PR, but I need @one-more-minute to flip travis on for this repo

@EricForgy
Copy link

I can also give a try here I guess: https://travis-ci.org/EricForgy/Blink.jl ?

PS: It's running :D https://travis-ci.org/EricForgy/Blink.jl/builds

PPS: Test failed. Was worth a try :)

@EricForgy
Copy link

@spencerlyon2 , ohhh. This is my first foray into Travis. I see the linux tests failed, but OSX passed. That is a good sign :) I am trying something new now.

@EricForgy
Copy link

Hi @spencerlyon2 ,

It is interesting that the OSX tests are passing and not Linux. I just checked and the OSX tests still pass even without "show = false:, so that is not the problem.

I'm installing VirtualBox so I can have Julia on Linux, but I am no expert so do not have high expectations :)

@EricForgy
Copy link

@spencerlyon2 @one-more-minute ,

Would it make sense to turn on tests only for OSX for now since those tests are passing and figure out what is wrong on linux later? At least we'd have some test working and we can start adding to it.

image

@EricForgy EricForgy mentioned this pull request Dec 29, 2015
@sglyon
Copy link
Contributor Author

sglyon commented Dec 29, 2015

My goal with the tests was to make sure that blink loaded and an electron window could be opened on all platforms.

I think that because we don't have any other tests (yet), we shouldn't merge until we figure out how to run tests on Windows and Linux.

Any other thoughts?

@EricForgy
Copy link

That makes sense, but I think Blink may not even work on Linux at the moment. Has anyone been able to get Blink to work on Linux? I'm on Windows, so I know it works on Windows and OSX (from Travis), but the very basic test fails on Linux and @tbreloff could not open a Window in #22 and he was on Linux.

It'd be great to start building some tests, even if just for OSX and Windows until someone comes along and gets things working on Linux.

@sglyon
Copy link
Contributor Author

sglyon commented Dec 29, 2015

I do have people using Blink on Ubuntu without problems. Not sure about other distros.

My vote would be to merge this under one of two conditions:

  1. We get all 3 platforms to pass the test
  2. We have other tests we want CI to track for platforms that currently do work

@tkelman
Copy link
Contributor

tkelman commented Jan 8, 2016

Try with xvfb-run on Linux.

@sglyon
Copy link
Contributor Author

sglyon commented Jan 8, 2016

what do you mean? Try running what with that command?

@sglyon
Copy link
Contributor Author

sglyon commented Jan 8, 2016

@tkelman could this potentially help: http://rhysd.hatenablog.com/entry/2015/08/07/181418

Maybe it is what you had in mind. I can try to give it a shot later

@EricForgy
Copy link

Drum roll :)

image

@EricForgy
Copy link

It worked! My tests are passing for linux! :)

But realized I was using the old yml. Will try again with Spencer's updated one...

@tkelman
Copy link
Contributor

tkelman commented Jan 9, 2016

I'm not sure if xvfb-run is going to work for mac, might need to set an env var TESTCMD="xvfb-run julia" for linux and TESTCMD="julia" for osx or something like that

@EricForgy
Copy link

Yeah. My tests broke :)

May give that a try, but learning Travis is not high on my priority list at the moment. I may need to wait for you guys, but thank's for the suggestion 👍 :)

@tkelman
Copy link
Contributor

tkelman commented Jan 9, 2016

Something like

matrix:
  include:
    - os: linux
      env: TESTCMD="xvfb-run julia"
    - os: osx
      env: TESTCMD="julia"
script:
  - if [[ -a .git/shallow ]]; then git fetch --unshallow; fi
  - $TESTCMD -e 'Pkg.clone(pwd()); Pkg.build("Blink"); Pkg.test("Blink"; coverage=true)'

@EricForgy
Copy link

Cool. Will try 💪

@EricForgy
Copy link

Before your suggestion, I tried:

# Documentation: http://docs.travis-ci.com/user/languages/julia/
language: julia
os:
  - linux
  - osx
julia:
  - release
  - nightly
notifications:
  email: false
# uncomment the following lines to override the default test script
env:
  - TESTCMD: "xvfb-run julia"
  - TESTCMD: "julia"
script:
  - if [[ -a .git/shallow ]]; then git fetch --unshallow; fi
  - $TESTCMD -e 'Pkg.clone(pwd()); Pkg.build("Plotly"); Pkg.test("Plotly"; coverage=true)'

Will see how that goes and try the matrix idea

Edit: Oops. Wrong package name when I uncommented. Trying again :)

@EricForgy
Copy link

Here's my latest yml:

# Documentation: http://docs.travis-ci.com/user/languages/julia/
language: julia
julia:
  - release
  - nightly
notifications:
  email: false
# uncomment the following lines to override the default test script
matrix:
  include:
    - os: linux
      env: TESTCMD="xvfb-run julia"
    - os: osx
      env: TESTCMD="julia"
script:
  - if [[ -a .git/shallow ]]; then git fetch --unshallow; fi
  - $TESTCMD -e 'Pkg.clone(pwd()); Pkg.build("Blink"); Pkg.test("Blink"; coverage=true)'

Still not passing.

@tkelman
Copy link
Contributor

tkelman commented Jan 9, 2016

Ahk, the travis matrix stuff is finicky - you either need to put everything in the matrix, or nothing. I'll open a PR against this branch.

@EricForgy
Copy link

Yay! That is awesome @tkelman 💪

With your yml #47, we have tests, including opening a Window, passing on both OSX and Linux 🎉

image

This is 2/3 of Spencer's 1st condition before merging :)

Next is Windows tests...

@tkelman
Copy link
Contributor

tkelman commented Jan 9, 2016

@EricForgy
Copy link

Ok. I added the example to my fork and changed "Example" -> "Blink" and signed up at Appveyor. Finger's crossed :)

@EricForgy
Copy link

@tkelman
Copy link
Contributor

tkelman commented Jan 9, 2016

Blink doesn't support 0.3 any more so you can remove the two JULIAVERSION lines for 0.3. The 0.4 versions of JuliaWeb packages should all be on MbedTLS rather than GnuTLS.

@EricForgy
Copy link

👍 Trying again. Thanks

@EricForgy
Copy link

So far so good:

image

use xvfb-run on Linux Travis
@EricForgy EricForgy mentioned this pull request Jan 9, 2016
@sglyon
Copy link
Contributor Author

sglyon commented Jan 9, 2016

@one-more-minute, can you flip on Travis and appveyor for this repo? I think someone with admin privileges has to do it.

Then I think we are good to go here, is that right @EricForgy?

@EricForgy
Copy link

As far as I can tell, yes :)

image

@MikeInnes
Copy link
Contributor

CI enabled – Nice work everybody!

sglyon added a commit that referenced this pull request Jan 9, 2016
@sglyon sglyon merged commit e71dc11 into master Jan 9, 2016
@tkelman
Copy link
Contributor

tkelman commented Jan 9, 2016

AppVeyor failed? Would be good to add status badges.

@sglyon
Copy link
Contributor Author

sglyon commented Jan 11, 2016

Agreed that we should have badges. I don't know where to find the AppVeyor results, do you have a link?

@tkelman
Copy link
Contributor

tkelman commented Jan 11, 2016

http://www.appveyor.com/docs/status-badges
see "for projects with public repositories"

@sglyon
Copy link
Contributor Author

sglyon commented Jan 12, 2016

Oh sorry, I meant I can't even find this repo on appveyor.

I looked here: https://ci.appveyor.com/project/JunoLab/blink-jl

but didn't find anything

@sglyon sglyon deleted the sl/dumb-test branch January 12, 2016 00:11
@tkelman
Copy link
Contributor

tkelman commented Jan 12, 2016

They don't really have org accounts last I checked, it'll be under whatever name @one-more-minute registered under. The badge can reference the github path though.

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