-
Notifications
You must be signed in to change notification settings - Fork 160
AC-1156: Move custom eslint tests to magento-coding-standard repo #262
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,9 @@ jobs: | |
name: Tests with PHP ${{ matrix.php-version }} and ${{ matrix.dependencies }} dependencies | ||
|
||
steps: | ||
- name: Setup node | ||
uses: actions/setup-node@v2 | ||
|
||
- uses: actions/checkout@v2 | ||
|
||
- name: Setup PHP | ||
|
@@ -34,6 +37,11 @@ jobs: | |
env: | ||
COMPOSER_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
||
- name: Install dependencies | ||
run: npm install | ||
- name: Run ESLint | ||
run: npm run eslint -- eslint/rules | ||
Comment on lines
+40
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should be separated into a different workflow to run the tests on different nodejs versions, also I don't think we should run eslint tests on different PHP versions |
||
|
||
- name: Validate composer | ||
run: composer validate | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
/vendor/* | ||
/bin/* | ||
/node_modules | ||
|
||
# IDE files | ||
.idea/ | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,34 @@ | ||||||
<?php | ||||||
/** | ||||||
* Copyright © Magento, Inc. All rights reserved. | ||||||
* See COPYING.txt for license details. | ||||||
*/ | ||||||
declare(strict_types=1); | ||||||
|
||||||
namespace Magento2\Tests\Eslint; | ||||||
|
||||||
use PHPUnit\Framework\TestCase; | ||||||
|
||||||
/** | ||||||
* Abstract class AbstractEslintTestCase | ||||||
* | ||||||
* Test Eslint Rules (magento-coding-standard/eslint/rules) | ||||||
*/ | ||||||
abstract class AbstractEslintTestCase extends TestCase | ||||||
{ | ||||||
/** | ||||||
* @param string $testFile | ||||||
* @param array $expectedMessages | ||||||
*/ | ||||||
protected function assertFileContainsError(string $testFile, array $expectedMessages) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you please add strict types and phpdocs to the added tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be done, please check it out! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
{ | ||||||
exec( | ||||||
'npm run eslint -- Magento2/Tests/Eslint/' . $testFile, | ||||||
$output | ||||||
); | ||||||
|
||||||
foreach ($expectedMessages as $message) { | ||||||
$this->assertStringContainsString($message, implode(' ',$output)); | ||||||
} | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
$(document).ready(function() { | ||
'use strict'; | ||
$("div").find("p").andSelf().addClass("border"); | ||
}); |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,24 @@ | ||||||
<?php | ||||||
/** | ||||||
* Copyright © Magento, Inc. All rights reserved. | ||||||
* See COPYING.txt for license details. | ||||||
*/ | ||||||
declare(strict_types=1); | ||||||
|
||||||
namespace Magento2\Tests\Eslint; | ||||||
|
||||||
/** | ||||||
* Class AndSelfTest | ||||||
* | ||||||
* Test Eslint Rule: jquery-no-andSelf.js | ||||||
*/ | ||||||
class AndSelfTest extends AbstractEslintTestCase | ||||||
{ | ||||||
public function testExecute() | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And remaining functions as well
Suggested change
|
||||||
{ | ||||||
$this->assertFileContainsError( | ||||||
'AndSelfTest.js', | ||||||
['jQuery.andSelf() removed, use jQuery.addBack()'] | ||||||
); | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
$(document).ready(function(){ | ||
'use strict'; | ||
$(".btn1").bind("click"); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento2\Tests\Eslint; | ||
|
||
/** | ||
* Class BindUnbindTest | ||
* | ||
* Test Eslint Rule: jquery-no-bind-unbind.js | ||
*/ | ||
class BindUnbindTest extends AbstractEslintTestCase | ||
{ | ||
public function testExecute() | ||
{ | ||
$this->assertFileContainsError( | ||
'BindUnbindTest.js', | ||
['jQuery $.bind and $.unbind are deprecated, use $.on and $.off instead'] | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
$(document).ready(function(){ | ||
'use strict'; | ||
$("input").blur(); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento2\Tests\Eslint; | ||
|
||
/** | ||
* Class ClickEventShorthandTest | ||
* | ||
* Test Eslint Rule: jquery-no-click-event-shorthand.js | ||
*/ | ||
class ClickEventShorthandTest extends AbstractEslintTestCase | ||
{ | ||
public function testExecute() | ||
{ | ||
$this->assertFileContainsError( | ||
'ClickEventShorthand.js', | ||
['Instead of .blur(fn) use .on("blur", fn). Instead of .blur() use .trigger("blur")'] | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
$(document).ready(function(){ | ||
'use strict'; | ||
$( "table" ).delegate( "td", "click", function() { | ||
$( this ).toggleClass( "chosen" ); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento2\Tests\Eslint; | ||
|
||
/** | ||
* Class DelegateUndelegateTest | ||
* | ||
* Test Eslint Rule: jquery-no-delegate-undelegate.js | ||
*/ | ||
class DelegateUndelegateTest extends AbstractEslintTestCase | ||
{ | ||
public function testExecute() | ||
{ | ||
$this->assertFileContainsError( | ||
'DelegateUndelegateTest.js', | ||
['jQuery $.delegate and $.undelegate are deprecated, use $.on and $.off instead'] | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
$(document).ready(function(){ | ||
'use strict'; | ||
$( "#result" ).load( "ajax/test.html" ); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento2\Tests\Eslint; | ||
|
||
/** | ||
* Class EventShorthandTest | ||
* | ||
* Test Eslint Rule: jquery-no-event-shorthand.js | ||
*/ | ||
class EventShorthandTest extends AbstractEslintTestCase | ||
{ | ||
public function testExecute() | ||
{ | ||
$this->assertFileContainsError( | ||
'EventShorthandTest.js', | ||
['jQuery.load() was removed, use .on("load", fn) instead'] | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
$(document).ready(function(){ | ||
'use strict'; | ||
$("div").size(); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento2\Tests\Eslint; | ||
|
||
/** | ||
* Class SizeTest | ||
* | ||
* Test Eslint Rule: jquery-no-size.js | ||
*/ | ||
class SizeTest extends AbstractEslintTestCase | ||
{ | ||
public function testExecute() | ||
{ | ||
$this->assertFileContainsError( | ||
'SizeTest.js', | ||
['jQuery.size() removed, use jQuery.length'] | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
$(document).ready(function(){ | ||
'use strict'; | ||
$.trim(" hello, how are you? "); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento2\Tests\Eslint; | ||
|
||
/** | ||
* Class TrimTest | ||
* | ||
* Test Eslint Rule: jquery-no-trim.js | ||
*/ | ||
class TrimTest extends AbstractEslintTestCase | ||
{ | ||
public function testExecute() | ||
{ | ||
$this->assertFileContainsError( | ||
'TrimTest.js', | ||
['jQuery.trim is deprecated; use String.prototype.trim'] | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"extends": [ | ||
"./.eslintrc-reset", | ||
"./.eslintrc-magento", | ||
"./.eslintrc-jquery", | ||
"./.eslintrc-misc" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"rules": { | ||
"jquery-no-andSelf": 2, | ||
"jquery-no-bind-unbind": 2, | ||
"jquery-no-click-event-shorthand": 2, | ||
"jquery-no-delegate-undelegate": 2, | ||
"jquery-no-event-shorthand": 2, | ||
"jquery-no-size": 2, | ||
"jquery-no-trim": 2 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From readme file:

https://github.com/actions/setup-node