Skip to content

Commit

Permalink
Fix oppia#10552: Add test coverage for eslint checks (oppia#11024)
Browse files Browse the repository at this point in the history
* Temp

* temp

* fail on exception

* Fixed yarn file

* Fixed coverage
  • Loading branch information
Hudda authored Nov 4, 2020
1 parent 1bb5107 commit 30cf57c
Show file tree
Hide file tree
Showing 10 changed files with 310 additions and 53 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ node_modules/*
coverage.xml
.viminfo
.vscode/*
.nyc_output/*
libpeerconnection.log
readme_test_dir/*
tsc_output_log.txt
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
"karma-webpack": "^4.0.0-rc.3",
"loader-utils": "^2.0.0",
"mocha": "^8.1.1",
"nyc": "^15.1.0",
"protractor": "^7.0.0",
"protractor-jasmine2-screenshot-reporter": "^0.5.0",
"puppeteer": "^5.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ module.exports = {
}
if (paren.value === '(') {
parensCount += 1;
} else if (paren.value === ')') {
} else {
if (parensCount > 0) {
parensCount -= 1;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ ruleTester.run('break-after-parens', rule, {
`it('should' +
'happen')`,
`angular.module('oppia').constant('default',
false);`
false);`,
`var a = (
true);`
],

invalid: [
Expand Down
73 changes: 33 additions & 40 deletions scripts/linters/custom_eslint_checks/rules/dependency-checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,6 @@ module.exports = {
const sourceCode = context.getSourceCode();

var isEquals = function(sortedImports, params) {
if (sortedImports === params) {
return true;
}
if (sortedImports === null || params === null) {
return false;
}
if (sortedImports.length !== params.length) {
return false;
}

for (var i = 0; i < sortedImports.length; ++i) {
if (sortedImports[i] !== params[i]) {
return false;
Expand Down Expand Up @@ -105,38 +95,41 @@ module.exports = {
ArrayExpression: function checkDirective(node) {
var paramsList = [];
var tokensList = [];
if (node.parent.key) {
if (node.parent.key.name === 'controller' &&
node.parent.parent.type === 'ObjectExpression') {
if (node.elements[node.elements.length - 1].type ===
'FunctionExpression') {
var params = node.elements[node.elements.length - 1].params;
for (var index in params) {
var param = params[index];
paramsList.push(param.name);
if (!node.parent.key) {
return true;
}
if (!(node.parent.key.name === 'controller' &&
node.parent.parent.type === 'ObjectExpression')) {
return true;
}
if (!(node.elements[node.elements.length - 1].type ===
'FunctionExpression')) {
return true;
}
var params = node.elements[node.elements.length - 1].params;
for (var index in params) {
var param = params[index];
paramsList.push(param.name);
}
var tokens = sourceCode.getTokens(node);
tokens.forEach((token) => {
if (token.type !== 'Punctuator') {
tokensList.push(token.value);
}
});
checkSortedDependency(node, paramsList);
paramsList.forEach((param) => {
if (isUnused(param, tokensList)) {
context.report({
node,
loc: node.loc,
messageId: 'unusedDirective',
data: {
dependencyName: param
}
var tokens = sourceCode.getTokens(node);
tokens.forEach((token) => {
if (token.type !== 'Punctuator') {
tokensList.push(token.value);
}
});
checkSortedDependency(node, paramsList);
paramsList.forEach((param) => {
if (isUnused(param, tokensList)) {
context.report({
node,
loc: node.loc,
messageId: 'unusedDirective',
data: {
dependencyName: param
}
});
}
});
}
});
}
}
});
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,56 @@ ruleTester.run('no-unused-dependency', rule, {
}
]
};
}`,
`function test() {
return {
controllers: [
'$http', '$translate', 'I18nLanguageCodeService',
'UserService', 'SUPPORTED_SITE_LANGUAGES',
function(
$http, $translate, I18nLanguageCodeService,
UserService, SUPPORTED_SITE_LANGUAGES) {
var ctrl = this;
// Changes the language of the translations.
var preferencesDataUrl = '/preferenceshandler/data';
var siteLanguageUrl = '/save_site_language';
ctrl.changeLanguage = function() {
$translate.use(ctrl.currentLanguageCode);
I18nLanguageCodeService.setI18nLanguageCode(
ctrl.currentLanguageCode);
UserService.getUserInfoAsync().then(function(userInfo) {
if (userInfo.isLoggedIn()) {
$http.put(siteLanguageUrl, {
site_language_code: ctrl.currentLanguageCode
});
}
});
};
ctrl.$onInit = function() {
ctrl.supportedSiteLanguages = SUPPORTED_SITE_LANGUAGES;
ctrl.currentLanguageCode = (
$translate.proposedLanguage() || $translate.use());
I18nLanguageCodeService.setI18nLanguageCode(
ctrl.currentLanguageCode);
};
}
]
};
}`,
`function test() {
return {
controller: [
(function() {
return true;
}), 'abc', 'abca'
]
};
}`,
`function test() {
a = [
'$http', '$translate', 'I18nLanguageCodeService',
'UserService', 'SUPPORTED_SITE_LANGUAGES'
]
}`
],

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,10 @@ module.exports = {
};

var extractMessage = function(node) {
if (node.type === 'Literal') {
return node.value;
} else if (node.type === 'BinaryExpression') {
if (node.type === 'BinaryExpression') {
return extractMessage(node.left) + extractMessage(node.right);
} else {
return node.value;
}
};

Expand Down
1 change: 1 addition & 0 deletions scripts/linters/pre_commit_linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,7 @@ def main(args=None):

errors_stacktrace = concurrent_task_utils.ALL_ERRORS
if errors_stacktrace:
failed = True
_print_errors_stacktrace(errors_stacktrace)

if failed:
Expand Down
13 changes: 12 additions & 1 deletion scripts/run_custom_eslint_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from __future__ import unicode_literals # pylint: disable=import-only-modules

import os
import re
import subprocess
import sys

Expand All @@ -30,9 +31,10 @@
def main():
"""Run the tests."""
node_path = os.path.join(common.NODE_PATH, 'bin', 'node')
nyc_path = os.path.join('node_modules', 'nyc', 'bin', 'nyc.js')
mocha_path = os.path.join('node_modules', 'mocha', 'bin', 'mocha')
filepath = 'scripts/linters/custom_eslint_checks/rules/'
proc_args = [node_path, mocha_path, filepath]
proc_args = [node_path, nyc_path, mocha_path, filepath]

proc = subprocess.Popen(
proc_args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
Expand All @@ -54,6 +56,15 @@ def main():
python_utils.PRINT('All tests passed')
python_utils.PRINT('---------------------------')

coverage_result = re.search = re.search(
r'All files\s*\|\s*(?P<stmts>\S+)\s*\|\s*(?P<branch>\S+)\s*\|\s*'
r'(?P<funcs>\S+)\s*\|\s*(?P<lines>\S+)\s*\|\s*', tests_stdout)
if (coverage_result.group('stmts') != '100' or
coverage_result.group('branch') != '100' or
coverage_result.group('funcs') != '100' or
coverage_result.group('lines') != '100'):
raise Exception('Eslint test coverage is not 100%')


if __name__ == '__main__':
main()
Loading

0 comments on commit 30cf57c

Please sign in to comment.