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

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

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

Conversation

GabLeRoux
Copy link

@GabLeRoux GabLeRoux commented Feb 27, 2018

Build Status

I didn't go in too much details, but I managed to get the CI working on this project. I suppose this can be improved, but at least if it's there, someone else may improve the tests for this project 👍

Owner (@macek macek) needs to enable travis-ci project on travis-ci.org. Then, we can also add this to the readme:

[![Build Status](https://travis-ci.org/macek/jquery-serialize-object
.svg?branch=master)](https://travis-ci.org/macek/jquery-serialize-object)

See results here:
https://travis-ci.org/GabLeRoux/jquery-serialize-object

this still needs some more work, help would be appreciated.
Here's an example project with mocha running with phantom in travis:
https://github.com/jh3y/testPlayground

And a related blog post:
https://medium.com/caffeine-and-testing/getting-started-with-mocha-bfa20d403186

I don't have much time right now to continue working on this, but I think it's a good starting point.

🎉

@GabLeRoux GabLeRoux changed the title Add travis-ci support to this project WIP: Add travis-ci support to this project Feb 27, 2018
@GabLeRoux
Copy link
Author

Currently failing with this:

...
The command "yarn run test" exited with 0.
$ mocha-phantomjs ./test/test.html
Likely due to external resource loading and timing, your tests require calling `window.initMochaPhantomJS()` before calling any mocha setup functions. See https://github.com/nathanboktae/mocha-phantomjs-core/issues/12
The command "mocha-phantomjs ./test/test.html" exited with 1.

and this

$ yarn run spec
yarn run v1.3.2
$ mocha -u tdd --reporter spec
No test files found
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
The command "yarn run spec" exited with 1.

.travis.yml Outdated
- 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

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

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

@GabLeRoux
Copy link
Author

GabLeRoux commented Feb 28, 2018

I applied suggested changes, It is still failing on the ci side with this:

$ mocha-phantomjs ./test/test.html
SyntaxError: Parse error

As the file doesn't get generated anymore (it was created by the test command).

For the test part, maybe we can duplicate the test entry in package.json to something like test-ci where open is simply not there. And then we could do both commands with the CI.

  1. build
  2. test (with generated test.html and mocha).

If test passes, we can also deploy artifacts when ran on master which would ease the release of new versions.

For the mocha part, I may need some help and time. I'll have a look at this in the next weeks.

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.

2 participants