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

Setup jest #130

Merged
merged 4 commits into from Aug 3, 2018
Merged

Setup jest #130

merged 4 commits into from Aug 3, 2018

Conversation

li-boxuan
Copy link
Member

Closes #45 & #129

@TravisBuddy
Copy link

Travis tests have failed

Hey @li-boxuan,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

Node.js: undefined

npm run test
> gh-board@0.0.0 test /home/travis/build/coala/gh-board
> ./script/run-test.sh

+files=("issues.json" "recent-issues.json")
+for file in '"${files[@]}"'
+[[ -f issues.json ]]
+echo 'File issues.json exists.'
File issues.json exists.
+for file in '"${files[@]}"'
+[[ -f recent-issues.json ]]
+echo 'File recent-issues.json exists.'
File recent-issues.json exists.
++npm bin
+/home/travis/build/coala/gh-board/node_modules/.bin/jest
/home/travis/build/coala/gh-board/jest.config.js:30
  coverageDirectory: "coverage",
  ^^^^^^^^^^^^^^^^^

SyntaxError: Unexpected identifier
    at createScript (vm.js:80:10)
    at Object.runInThisContext (vm.js:139:10)
    at Module._compile (module.js:599:28)
    at Object.Module._extensions..js (module.js:646:10)
    at Module.load (module.js:554:32)
    at tryModuleLoad (module.js:497:12)
    at Function.Module._load (module.js:489:3)
    at Module.require (module.js:579:17)
    at require (internal/module.js:11:18)
    at exports.default.configPath (/home/travis/build/coala/gh-board/node_modules/jest-config/build/read_config_file_and_set_root_dir.js:44:20)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! gh-board@0.0.0 test: `./script/run-test.sh`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the gh-board@0.0.0 test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/travis/.npm/_logs/2018-07-29T04_27_38_705Z-debug.log

package.json Outdated
@@ -37,18 +37,22 @@
"babel-plugin-transform-class-properties": "^6.24.1",
"babel-plugin-transform-object-rest-spread": "^6.26.0",
"babel-preset-env": "^1.6.1",
"babel-preset-es2015": "^6.24.1",
Copy link
Member

Choose a reason for hiding this comment

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

env should cover for es2015 already

http://babeljs.io/docs/en/env/

.babelrc Outdated
@@ -0,0 +1,3 @@
{
"presets": ["es2015", "react"]
Copy link
Member

Choose a reason for hiding this comment

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

Extract this fom webpack conf

options: {
presets: [
'react',
'env'
],
plugins: [
'react-require',
'transform-object-rest-spread',
'transform-class-properties'
],

babel-loader will also read from .babelrc if it exists.

@@ -1,3 +1,4 @@
import React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this now since we have the same config as webpack?

@blazeu blazeu requested review from wisn, andrewda and jayvdb July 29, 2018 12:12
@li-boxuan li-boxuan force-pushed the setup_jest branch 2 times, most recently from 85fac2a to d609707 Compare July 29, 2018 12:38
@jayvdb
Copy link
Member

jayvdb commented Jul 29, 2018

Some general thoughts:

I believe .babelrc stuff can be put into package.json .. I prefer that, but what about other ppl?

codecov.yml is currently doing nothing, so isnt necessary. Data can be sent to codecov without it, and the percentage will slowly rise. until it gets to a decent percentage, tracking that isnt useful.

travis after_success/after_failure needs to be deactivated for the moban job.

jest.config.js - I am not a fan of checking in a large config file of comments about possible settings. We know whether to find the docs if we want more settings.

Copy link
Member

@jayvdb jayvdb left a comment

Choose a reason for hiding this comment

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

see comment above

Copy link
Member

@wisn wisn left a comment

Choose a reason for hiding this comment

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

Well, after all those reviews above this looks good to me.

@wisn
Copy link
Member

wisn commented Jul 29, 2018

@jayvdb I would prefer .babelrc than package.json

@jayvdb
Copy link
Member

jayvdb commented Jul 30, 2018

@wisn, why?

@wisn
Copy link
Member

wisn commented Jul 30, 2018

@jayvdb I knew that several peoples hate "tiny" file like .babelrc just for storing some configurations. If that's the case then we could merge Babel configuration to package.json. My reason to use .babelrc is to explicitly say that "this application using Babel and here is the configuration". Besides, package.json contains a lot of dependencies record already and I see it as a mess. Actually, based on this repo behavior and Babel lookup behavior, move Babel configuration to package.json would be good. Hence, we just need to make a deal since either way is literally the same.

How about you guys? @li-boxuan @andrewda @blazeu

@jayvdb jayvdb requested a review from nkprince007 July 30, 2018 04:50
.babelrc Outdated
@@ -0,0 +1,11 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

This change is welcoming. Most of the text editors these days automatically pick up babelrc files and provide autocompletion features based on babel configuration.

.codecov.yml Outdated
@@ -0,0 +1,9 @@
comment: false
Copy link
Member

Choose a reason for hiding this comment

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

Like @jayvdb mentioned, we don't need a configuration to publish code coverage to codecov.

.travis.yml Outdated
@@ -18,18 +20,33 @@ jobs:
before_script: false
script: .ci/check_moban.sh
after_success: false
after_failure: false
Copy link
Member

Choose a reason for hiding this comment

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

please add a disable_global variable like https://github.com/coala/git-task-list/blob/master/.travis.yml#L13 , so all extra jobs can re-use it.

.travis.yml Outdated
@@ -8,6 +8,8 @@ cache:
# env forces jobs to be created from the top level settings
env:
jobs:
allow_failures:
- script: script/run-integration-test.sh
Copy link
Member

Choose a reason for hiding this comment

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

I am very surprised this works.
Please use a variable like we do in coala/coala repo for the job which is allowed to fail

@jayvdb
Copy link
Member

jayvdb commented Aug 3, 2018

a SyntaxError with the integration test

Move integration test into individual stage and allow failure for it.

Related to coala#45
@jayvdb
Copy link
Member

jayvdb commented Aug 3, 2018

ack 59e9cba 8a3c49b 1f88cfa 3638063

@jayvdb
Copy link
Member

jayvdb commented Aug 3, 2018

@gitmate-bot ff

@gitmate-bot
Copy link

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

@gitmate-bot gitmate-bot merged commit 3638063 into coala:master Aug 3, 2018
@gitmate-bot
Copy link

Automated fastforward with GitMate.io was successful! 🎉

@li-boxuan li-boxuan deleted the setup_jest branch August 3, 2018 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

7 participants