Add test task to TravisCI, use yarn

[byzyk]

as per discussion in #2224

[montogeek]

Why this change?

[byzyk]

install happens before the script, so it makes more sense to move script at the end, where it actually takes place. Doesn't really affect functionality, just to make config more clear to read

[jeremenichelli]

I think it's not necessary to install yarn since Travis detects the yarn.lock file and does it already,

[jeremenichelli]

I would also wait til your fixes on linting are merged so you can reorganize the npm scripts to to run the checks. It might also be a good opportunity to add a pre-commit hook so we don't contributors don't find out about them when the CI fails, which is a poor developer experience.

[byzyk]

Good catch on yarn.lock file!

Just to clarify, by pre-commit hooks you mean add linter with --fix flag to automatically fix possible style issues?

[byzyk]

Or we could simply run yarn test there, yeah now I get it. Will get my hands on it tomorrow

[montogeek]

Please do it in another PR, it is better to keep them small.

[byzyk]

Sure 👍

[jeremenichelli]

Yeha good point Fernando.

About the precommit hook, I wouldn't do it silently, better prevent a commit un til the dev fixes the issues since --fix flags usually can't cover all cases.

[jeremenichelli]

@montogeek let me know if you have any other concern on this so we can merge.

[jeremenichelli]

Hey @byzyk just being curious. If you go to Configuration index page, you will see that we have a js with links type of code snippet. Do you know how this plus your previous work on linting code tags would work? Is it ignoring it or actually going through it?

[byzyk]

@jeremenichelli good question. Linter will ignore that code since eslint-plugin-markdown supports only these code types: js, javascript, node, jsx. If we wanna run linter on js-with-links stuff we would have to write a parser for it ourselves since it's a mixture of JS and some other tags.

[byzyk]

Seems like it throws on linter error in code examples 😃

[jeremenichelli]

Yeah that's good news. I did a rebase on purpose to make sure that we fix possible insertions during these days so future PRs don't have to fix others lint problems.

Won't you mind addressing them before merging? And thanks for all your work on this, looks amazing!

[byzyk]

Thanks! Glad it worked.

I fixed remaining linter issues, now CI should pass.

[dhruvdutt]

I hope the change in order between test and build doesn't break anything.

[byzyk]

Why should it break anything? On the other hand, we want to prevent build if tests are failing

[montogeek]

Thank you

[montogeek]

This PR doesn't work, you added it to deploy.sh, not to each Travis job. Will do it on #2251.

Edit:
It is running, but still not working, when running locally it found a lot of issues.

[byzyk]

@montogeek can you elaborate a bit more? It works for me locally, on which branch are you trying it?

[montogeek]

@byzyk master.
Run
yarn fetch
yarn lint

[byzyk]

@montogeek all ok, could you share your output? Please note that I've added generated folder to .eslintignore file.

See my output:

webpack.js.org[master] ➤  yarn fetch
yarn run v1.7.0
$ sh src/scripts/fetch.sh
Fetched 40 files: webpack-contrib/json-loader,webpack-contrib/raw-loader,webpack-contrib/coffee-loader,webpack-contrib/css-loader,webpack-contrib/style-loader,webpack-contrib/script-loader,webpack-contrib/less-loader,webpack-contrib/bundle-loader,webpack-contrib/val-loader,webpack-contrib/file-loader,webpack-contrib/url-loader,webpack-contrib/i18n-loader,webpack-contrib/json5-loader,webpack-contrib/worker-loader,webpack-contrib/jshint-loader,webpack-contrib/imports-loader,webpack-contrib/exports-loader,webpack-contrib/mocha-loader,webpack-contrib/coverjs-loader,webpack-contrib/expose-loader,webpack-contrib/node-loader,webpack-contrib/coffee-redux-loader,webpack-contrib/transform-loader,webpack-contrib/html-loader,webpack-contrib/sass-loader,webpack-contrib/source-map-loader,webpack-contrib/react-proxy-loader,webpack-contrib/null-loader,webpack-contrib/multi-loader,webpack-contrib/istanbul-instrumenter-loader,webpack-contrib/eslint-loader,webpack-contrib/yaml-frontmatter-loader,webpack-contrib/svg-inline-loader,webpack-contrib/restyle-loader,webpack-contrib/gzip-loader,webpack-contrib/cache-loader,webpack-contrib/thread-loader,webpack-contrib/polymer-webpack-loader,webpack-contrib/workerize-loader,webpack-contrib/config-loader
Fetched 1 file: babel/babel-loader
Fetched 1 file: postcss/postcss-loader
Fetched 1 file: peerigon/extract-loader
Fetched 12 files: webpack-contrib/i18n-webpack-plugin,webpack-contrib/component-webpack-plugin,webpack-contrib/compression-webpack-plugin,webpack-contrib/extract-text-webpack-plugin,webpack-contrib/copy-webpack-plugin,webpack-contrib/npm-install-webpack-plugin,webpack-contrib/stylelint-webpack-plugin,webpack-contrib/babel-minify-webpack-plugin,webpack-contrib/uglifyjs-webpack-plugin,webpack-contrib/zopfli-webpack-plugin,webpack-contrib/closure-webpack-plugin,webpack-contrib/css-webpack-plugin
Fetched 1 file: webpack-contrib/mini-css-extract-plugin
Fetched 1 file: support-backers.json
Fetched 1 file: starter-kits-data.json
✨  Done in 34.78s.
webpack.js.org[master] ➤  yarn lint
yarn run v1.7.0
$ run-s lint:*
$ eslint . --ext .js,.jsx,.md
$ markdownlint --config ./.markdownlint.json *.md ./src/content/**/*.md

$ alex . -q
$ cp .proselintrc ~/ && proselint src/content
✨  Done in 14.45s.

[montogeek]

This is the end :D

✖ 7003 problems (7003 errors, 0 warnings)
  6487 errors, 0 warnings potentially fixable with the `--fix` option.

[byzyk]

Really weird 😬 Where are most of the issues coming from? My guess that it's trying to lint generated folder which it shouldn't.

[montogeek]

How it doesn't try to eslint dist folder? I guess it doesn't exists in your case and also on Travis.

It also linting dynamically imported md files, which also doesn't happen on Travis because those MD file get written after test script runs.

So, lint issues only happen if you build the project locally, I did #2251 to explicitly set ESLint and Markdownlint to ignore those folders.

[byzyk]

Yeah true, I guess dist folder was introduced in rebuild branch, but I'm not sure.

Anyway, we shouldn't run linter on any dist folder (eg. dist, build, generated etc.) since linting should take place before building, right?

[montogeek]

Yeah, sorry dist on rebuild, build in master.
Yeah, it shouldn't.