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

Clean up a bit travis.yml #516

Merged
merged 1 commit into from
Aug 19, 2016
Merged

Conversation

AlexKVal
Copy link
Contributor

@AlexKVal AlexKVal commented Aug 11, 2016

Remove installation of npm poltergeist package. It seems a typo.

TravisCI's recommended way to run Xvfb https://docs.travis-ci.com/user/gui-and-headless-browsers/#Using-xvfb-to-Run-Tests-That-Require-a-GUI

Note: Don’t run xvfb directly, as it does not handle multiple concurrent instances that way.

The npm config set spin false speeds things a bit up.


This change is Reviewable

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 3d2e7d9 on AlexKVal:travis-config into * on shakacode:master*.

@justin808
Copy link
Member

@AlexKVal Thanks!

@dylangrafmyre You'd be better to review this me. If this looks good to you, let's merge this.


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@justin808
Copy link
Member

@dylangrafmyre Travis passed, FWIW.

@dylangrafmyre
Copy link
Contributor

@justin808 @AlexKVal I have not run the ci rake task locally for this repo for a while.
I do not know what is required for it to run successfully.

However, if it can pass by using PhantomJs and Poltergeist than maybe some of this .travis.yml before_install items can be deleted?

Sometime all of these things get patched together to make it work because of web driver prior failures with react,etc. Maybe now it might be worth stripping down the the basics in the .travis.yml and testing if the suite works with only phantomjs and poltergeist?

But if this is what it takes now to get CI passing that is fine too. Just sometimes you need to revisit the CI config because of changes to the node_modules and gems that have adapted and provide better support for our tool chain.


Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@dylangrafmyre
Copy link
Contributor

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@AlexKVal
Copy link
Contributor Author

AlexKVal commented Aug 12, 2016

Maybe now it might be worth stripping down the the basics in the .travis.yml and testing if the suite works with only phantomjs and poltergeist?

I've tried to run tests with PhantomJS instead of Chrome:
https://travis-ci.org/AlexKVal/react_on_rails/builds/151347102

PhantomJS crashes.
phantomjs

I did not do any additional research about it.

I've tried running of the tests locally with phantomjs@2.1.1. Everything is green.
But the TravisCI does not yet support phantomjs@2+ versions.

@AlexKVal AlexKVal force-pushed the travis-config branch 2 times, most recently from b871160 to 4b644ca Compare August 12, 2016 19:13
@coveralls
Copy link

coveralls commented Aug 12, 2016

Coverage Status

Coverage remained the same at 82.36% when pulling 4b644ca on AlexKVal:travis-config into a6373f0 on shakacode:master.

@AlexKVal
Copy link
Contributor Author

I've made "install" section a bit simpler and added caching.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.36% when pulling 4ff4656 on AlexKVal:travis-config into a6373f0 on shakacode:master.

@justin808
Copy link
Member

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


.travis.yml, line 34 [r2] (raw file):

install:
  - nvm install 5

@AlexKVal @dylangrafmyre Why not version 6? or just latest node or LTS:

https://github.com/creationix/nvm#usage


Comments from Reviewable

@dylangrafmyre
Copy link
Contributor

@AlexKVal Here is an example for getting phantomjs 2.1 + installed within .travis.yml
https://github.com/testdouble/present/blob/57deb323731c23607d4e3cafd709ec062d9b6666/.travis.yml


Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


.travis.yml, line 34 [r2] (raw file):

Previously, justin808 (Justin Gordon) wrote…

@AlexKVal @dylangrafmyre Why not version 6? or just latest node or LTS:

https://github.com/creationix/nvm#usage

It should be updated to version 6.

Comments from Reviewable

@AlexKVal
Copy link
Contributor Author

Thank you for the help. I'll check it out and report on it.


Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


.travis.yml, line 34 [r2] (raw file):

Previously, dylangrafmyre (Dylan Grafmyre) wrote…

It should be updated to version 6.

I wasn't sure if it is right to propose the node's version update. Then should I rewrite it as `nvm install 6` or `nvm install node` to always get the latest stable ?

Comments from Reviewable

@justin808
Copy link
Member

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


.travis.yml, line 34 [r2] (raw file):

Previously, AlexKVal (Alexander Shemetovsky) wrote…

I wasn't sure if it is right to propose the node's version update.
Then should I rewrite it as nvm install 6
or nvm install node to always get the latest stable ?

I think the latter. I want to know if one day we become incompatible with the latest.

Comments from Reviewable

@AlexKVal
Copy link
Contributor Author

AlexKVal commented Aug 17, 2016

I've just changed npm install node and added node -v.
My build is green: https://travis-ci.org/AlexKVal/react_on_rails/builds/152970530

As for the phantom:
TravisCI now has Phantom@2.0.0 by default.
It crashes.
I've tried the provided link with Phantom@2.1.1. It also crashes.
https://travis-ci.org/AlexKVal/react_on_rails/builds/152966747
2 1 1
crach

Remove installation of [npm poltergeist package](https://www.npmjs.com/package/poltergeist). It seems a typo.

--
`TravisCI`'s recommended way to run `Xvfb` https://docs.travis-ci.com/user/gui-and-headless-browsers/#Using-xvfb-to-Run-Tests-That-Require-a-GUI
> Note: Don’t run `xvfb` directly, as it does not handle multiple concurrent instances that way.

--
The [npm config set spin false](https://github.com/emberjs/ember.js/blob/86f65045fddd5fdd09685f39884230474dd2e111/.travis.yml#L14) speeds things a bit up.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.36% when pulling e14e8dc on AlexKVal:travis-config into ea95759 on shakacode:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.36% when pulling e14e8dc on AlexKVal:travis-config into ea95759 on shakacode:master.

@justin808
Copy link
Member

:lgtm:


Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@justin808 justin808 merged commit a4d9033 into shakacode:master Aug 19, 2016
@justin808
Copy link
Member

Thanks @AlexKVal!

@AlexKVal AlexKVal deleted the travis-config branch August 19, 2016 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants