JS Static tests added to CI (ESLint + JSCS) #9093
Conversation
| @@ -0,0 +1,18 @@ | |||
| { | |||
There was a problem hiding this comment.
Please, convert jscs config to json too.
| @@ -17,7 +17,7 @@ module.exports = function (grunt) { | |||
| }; | |||
|
|
|||
| grunt.registerTask('static', function (target) { | |||
| var currentTarget = target || 'test', | |||
| var currentTarget = target || 'file', | |||
There was a problem hiding this comment.
Currently we use grunt static to run all test with generating log file and grunt static:file --file=<path_to_file> for single file validation. What reason to switch this?
There was a problem hiding this comment.
UX. Dev should be able to quickly run grunt static to get some instant, readable feedback, without trying to find undocumented tasks options.
If you need to generate this XML files for some reason, run gulp static:test instead.
There was a problem hiding this comment.
Ok, make sense. But change, please, in dev/travis/script.sh grunt static to grunt static:test.
There was a problem hiding this comment.
But... the whole point is to not use test on CI, b/c output is saved only as JUnit XML file, which isn't available, b/c build artifacts are not saved anywhere (even if they will be publicly available, it's way more complicated way than just read the tests output).
Also, quiet option do more harm than good, b/c ignore all warnings.
Results of gulp static:test
Running "eslint:test" (eslint) task
Warning: Task "eslint:test" failed. Used --force, continuing.
Results of gulp static aka. gulp static:file
Running "eslint:file" (eslint) task
/Users/igloczek/Sites/magento2-dev/lib/web/mage/accordion.js
7:9 error Parsing error: Unexpected token 'jquery'
✖ 1 problem (1 error, 0 warnings)
Warning: Task "eslint:file" failed. Used --force, continuing.
dev/tools/grunt/tasks/static.js
Outdated
| @@ -26,7 +26,11 @@ module.exports = function (grunt) { | |||
|
|
|||
| setConfig('eslint', currentTarget, cvf.getFiles(file)); | |||
| setConfig('jscs', currentTarget, cvf.getFiles(file)); | |||
| grunt.option('force', true); | |||
|
|
|||
| if (grunt.option('force') !== false) { | |||
There was a problem hiding this comment.
Please, revert this changes, we already solve this issue. We add grunt-continue, that provide ability to run in force mode(for collect eslint and jscs errors) and fail on warnings. Internal ticket MAGETWO-67342 now in delivery process.
dev/travis/script.sh
Outdated
| static) | ||
| TEST_FILTER='--filter "Magento\\Test\\Php\\LiveCodeTest"' || true | ||
| phpunit -c dev/tests/$TEST_SUITE $TEST_FILTER | ||
| grunt static --force=false |
There was a problem hiding this comment.
Please, remove --force.
|
Hi @Igloczek |
|
@ishakhsuvarov I'm just busy as f... Let's keep it open, I'll finish this ASAP. |
|
@Igloczek Sure. No rush, just trying to make sure we are on the same page. |
ishakhsuvarov
changed the title
ishakhsuvarov
changed the title
|
@Igloczek Thank you for the contribution |
This PR is part of #9089.
I adjusted Travis config files to run
gulp statictask (ESLint + JSCS).I changed default
targetofstatictask tofile, b/c by previously all output was saved to XML files, that's not so useful on daily basics.Test are not green, b/c of reported JS issues.
PS. Upgrading dependencies (#9062) will increase number of reported issues 😉