Skip to content

WIP: Add travis-ci support to this project #119

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.

Sign up for GitHub

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

WIP: Add travis-ci support to this project #119

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4733fe9
Add travis-ci integration for running tests :tada:
GabLeRoux Feb 27, 2018
573346b
Add travis cache for yarn's node_modules :v:
GabLeRoux Feb 27, 2018
59048c0
Don't run 'open' in package.json's 'test' command for CI
GabLeRoux Feb 27, 2018
f314939
Fix .travis.yml syntax for 'script'
GabLeRoux Feb 27, 2018
3157f57
Use mocha-phantomjs to run the tests suite in travis
GabLeRoux Feb 27, 2018
8da2445
Attempt to run mocha spec
GabLeRoux Feb 27, 2018
3795cef
Add 'mocha' to travis build
GabLeRoux Feb 27, 2018
908a61f
Fix npm build and test scripts responsabilities
GabLeRoux Feb 28, 2018
b26b0b0
Add mocha to devDependencies
GabLeRoux Feb 28, 2018
72e435b
Run npm run build instead of test in travis
GabLeRoux Feb 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
language: node_js
node_js:
- "6.12.0"
- "node"
- "lts/*"
- "8"
- "7"
- "6"
- "5"
- "4"

cache: yarn

before_script:
- yarn global add bower
- bower -v
- bower i
- yarn global add phantomjs
- phantomjs --version
- yarn global add mocha
- mocha --version
- yarn global add mocha-phantomjs
- mocha-phantomjs --version

script:
- yarn run test
Copy link
Owner

Choose a reason for hiding this comment

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

this should just be yarn run build, then the test script doesn't have to be modified. is that correct?

Copy link
Owner

Choose a reason for hiding this comment

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

and don't forget to add Mocha to devDependencies in package.json

Copy link
Author

@GabLeRoux GabLeRoux Feb 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure what you mean by this:

this should just be yarn run build, then the test script doesn't have to be modified. is that correct?

Do you mean that this is travis's default behaviour so we could delete the script part? Since there are other commands after this one, I think this is necessary.

and don't forget to add Mocha to devDependencies in package.json

definitely 👍

Copy link
Owner

@macek macek Feb 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'm suggesting the following revisions

in travis.yml just run build directly

 script:
-  - yarn run test
+  - yarn run build
   - mocha-phantomjs ./test/test.html
   - yarn run spec

in package.json restore the original test script; there was really no reason to modify it

 "scripts": {
-  "test": "npm run build",
+  "test": "npm run build && open ./test/test.html",
   "spec": "mocha -u tdd --reporter spec",
   "build": "npm run minify",
   "minify": "uglifyjs jquery.serialize-object.js -m -c --comments > dist/jquery.serialize-object.min.js"
 },

...

 "devDependencies": {
+  "mocha": "^5.0.1",
   "uglify-js": "^2.4.16"
 }

Does that make sense?

Copy link
Owner

Choose a reason for hiding this comment

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

In hindsight, the build and minify scripts cross concerns. It's a separate issue but should be patched as

-  "build": "npm run minify",
+  "build": "npm run minify > dist/jquery.serialize-object.min.js",
-  "minify": "uglifyjs jquery.serialize-object.js -m -c --comments > dist/jquery.serialize-object.min.js"
+  "minify": "uglifyjs jquery.serialize-object.js -m -c --comments"

If you want to sneak that in this PR, I'm cool with that

- mocha-phantomjs ./test/test.html
- yarn run spec
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"description": "serialize form fields into an object or JSON",
"main": "./jquery.serialize-object.js",
"scripts": {
"test": "npm run build && open ./test/test.html",
"test": "npm run build",
"spec": "mocha -u tdd --reporter spec",
"build": "npm run minify",
"minify": "uglifyjs jquery.serialize-object.js -m -c --comments > dist/jquery.serialize-object.min.js"
},
Expand Down