Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Upgrade packages and tools for Travis unit testing #3262

Merged
merged 11 commits into from
Jul 16, 2017
Merged

Upgrade packages and tools for Travis unit testing #3262

merged 11 commits into from
Jul 16, 2017

Conversation

wimrijnders
Copy link
Contributor

@wimrijnders wimrijnders commented Jul 16, 2017 •
edited
Loading

This addresses the warnings and notifications that pop up in the job log of the travis-cl unit tests.
This job log has been used as a starting point. The goal is to resolve all warning/error messages (Update: The goal has failed).

Compare latest job log of this PR with the previously linked job log for results.

For review, please do an npm install from scratch and run the unit tests
It all works on my own machine and on travis-cl, but YMMV due to local platform differences.

Changes

  • upgrade compiler to C++11 as per these instructions
  • add canvas module and minimal required components (libgif). This module will be used for unit testing Network and Graph3D.
  • Upgraded versions of modules in packages.json as much as possible/necessary. This required some fixes in gulpfile.js for usage of webpack.
  • Small fix in Network due to linting

Notes

Gulp latest stable links to deprecated modules

During npm install the following warnings are displayed:

npm WARN deprecated minimatch@2.0.10: Please update to minimatch 3.0.2 or higher to avoid a RegExp Dos issue

(Also for minimatch@0.2.14)

npm WARN deprecated graceful-fs@1.2.3: graceful-fs v3.0.0 and before will fail on node releases >= v7.0. Please update to graceful-fs@^4.0.0 as soon as possible. Use 'npm ls graceful-fs' to find it in the tree.

Both of these warnings are traceable to gulp v3.9.1 (latest stable), which has not been updated wrt. these warnings. Try as I might, I can't force the versions. We'll just have to wait till the gulp developers fix this.

Update: The gulp developers are very much aware of the deprecation warnings, see for example their issue 1571. They regards this as a breaking change, and it will be fixed in gulp version 4.

Warning on webpack when running gulp

With the upgraded webpack v2.x.x, the following notice may appear when running gulp:

loaderUtils.parseQuery() received a non-string value which can be problematic, see webpack/loader-utils#56
parseQuery() will be replaced with getOptions() in the next major version of loader-utils.

Following the link, it is explained that this is a notice to the webpack-loader developers and can be ignored by the application developers.
(it sure is ugly, though)

Warnings on installing module fsevents

The following notification may appear:

npm WARN optional dep failed, continuing fsevents@1.1.2

It turns out that fsevents is OS-X specific, and is not required for running on other platforms. If you don't run on a Mac, you can safely ignore this message.

bradh
bradh previously approved these changes Jul 16, 2017
View reviewed changes
@wimrijnders wimrijnders self-assigned this Jul 16, 2017
yotamberk
yotamberk approved these changes Jul 16, 2017
View reviewed changes
Copy link
Contributor

@yotamberk yotamberk left a comment

Choose a reason for hiding this comment

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

Awesome work! Great explanations!

@yotamberk yotamberk merged commit 843eb44 into almende:develop Jul 16, 2017
@wimrijnders wimrijnders deleted the updateForTravis branch July 17, 2017 05:01
wimrijnders added a commit to wimrijnders/vis that referenced this pull request Jul 17, 2017
@wimrijnders
Fix missing commend end in .eslintrc
f909749 
Fix typo in almende#3262, ending `*/` of a block comment was missing.
Also disabled new item `no-useless-escape` for linting, because this triggers a bit too often for comfort.
This was referenced Jul 17, 2017
Merged
Closed
yotamberk pushed a commit that referenced this pull request Jul 20, 2017
@wimrijnders @yotamberk
Fix missing commend end in .eslintrc (#3264)
e698c5d 
Fix typo in #3262, ending `*/` of a block comment was missing.
Also disabled new item `no-useless-escape` for linting, because this triggers a bit too often for comfort.
mojoaxel
mojoaxel reviewed Oct 2, 2017
View reviewed changes
package.json
@@ -48,21 +51,25 @@
"babel-preset-es2015": "^6.6.0",
"babel-runtime": "^6.22.0",
"babelify": "^7.3.0",
"canvas": "^1.6.5",
Copy link
Member

Choose a reason for hiding this comment

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

@wimrijnders What do we need this for? This module requires Cairo wich is an unacceptable external dependency. Since this I can not install vis anymore on linux 😭

Copy link
Contributor Author

Choose a reason for hiding this comment

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

canvas is used to run unit tests on Network. Why is cairo an unacceptable external dependency?

Copy link
Member

@mojoaxel mojoaxel Oct 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

cairo can not be installed via npm and therefor adds a OS-specific dependency. How should the user (like me) know that he has to install g++ and libgif-dev? npm install now just fails. I personally think this is not acceptable!
We should not have dependencies other that npm (and maybe git) as far as I'm concerned.
The least thing would be to document this in the README.

Copy link
Member

@mojoaxel mojoaxel Oct 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I've created an issue for that: #3515 Lets discuss over there, what the best way is to solve this.

@mojoaxel mojoaxel mentioned this pull request Oct 2, 2017
Open
primozs pushed a commit to primozs/vis that referenced this pull request Jan 3, 2019
@wimrijnders
Upgrade packages and tools for Travis unit testing (almende#3262)
8f70018 
* Add C++11 and canvas module for travis tests

* Added libgif, updated version gulp-clean-css

* Update version webpack

* Force versions minimatch, graceful-fs; fixes for upgraded webpack

* Force version minimatch through travis.yml

* Fix comma's in json

* Add extraneous modules to package.json; final attempt at forcing versions of minimatch and graceful-fs

* Final changes module versions

* Fix due to linting

* Fix typo in package.json

* Upgrade eslint
primozs pushed a commit to primozs/vis that referenced this pull request Jan 3, 2019
@wimrijnders
Fix missing commend end in .eslintrc (almende#3264)
bd66218 
Fix typo in almende#3262, ending `*/` of a block comment was missing.
Also disabled new item `no-useless-escape` for linting, because this triggers a bit too often for comfort.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mojoaxel mojoaxel mojoaxel left review comments

@yotamberk yotamberk yotamberk approved these changes

@bradh bradh bradh left review comments

Assignees

@wimrijnders wimrijnders

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wimrijnders @bradh @mojoaxel @yotamberk