Skip to content

Commit

Permalink
Create rule S5973: Tests should be stable
Browse files Browse the repository at this point in the history
  • Loading branch information
kebetsi committed Dec 5, 2023
1 parent 451d6a3 commit abf27ba
Show file tree
Hide file tree
Showing 18 changed files with 374 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"file-for-rules:tests/S5973.ts": [
3
]
}
3 changes: 3 additions & 0 deletions its/sources/jsts/custom/tests/S5973.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import * as jest from 'jest';

jest.retryTimes(3); // Noncompliant
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
jest.retryTimes(3); // Noncompliant

foo(); // for coverage
36 changes: 36 additions & 0 deletions packages/jsts/src/rules/S5973/fixtures/with-jest/cb.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
import path from 'path';
import { getAllPackageJsons, searchPackageJsonFiles } from '../../../../';
import { check } from '../../../tools';
import { rule } from '../../';


const sonarId = path.basename(__dirname);

describe('Rule S5973', () => {
beforeEach(() => {
searchPackageJsonFiles(__dirname, []);
});
afterAll(() => {
getAllPackageJsons().clear();
});
check(sonarId, rule, __dirname);
});
15 changes: 15 additions & 0 deletions packages/jsts/src/rules/S5973/fixtures/with-jest/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "jest-global",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"keywords": [],
"author": "",
"license": "ISC",
"devDependencies": {
"jest": "29.7.0"
}
}
36 changes: 36 additions & 0 deletions packages/jsts/src/rules/S5973/fixtures/without-jest/cb.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
import path from 'path';
import { getAllPackageJsons, searchPackageJsonFiles } from '../../../../';
import { check } from '../../../tools';
import { rule } from '../../';


const sonarId = path.basename(__dirname);

describe('Rule S5973', () => {
beforeEach(() => {
searchPackageJsonFiles(__dirname, []);
});
afterAll(() => {
getAllPackageJsons().clear();
});
check(sonarId, rule, __dirname);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import * as jest from 'jest';

jest.retryTimes(3); // Noncompliant
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
describe('api.foo()', function () {
this.retries(2); // Noncompliant

it('should return 42', function () {
this.retries(5); // Noncompliant
});
it('should return 42', () => {
this.retries(5); // Noncompliant - FP, because Mocha callbacks with arrow functions lose context. But we accept this.
});
});
12 changes: 12 additions & 0 deletions packages/jsts/src/rules/S5973/fixtures/without-jest/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "without-jest",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"keywords": [],
"author": "",
"license": "ISC"
}
20 changes: 20 additions & 0 deletions packages/jsts/src/rules/S5973/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
export { rule } from './rule';
111 changes: 111 additions & 0 deletions packages/jsts/src/rules/S5973/rule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
// https://sonarsource.github.io/rspec/#/rspec/S5973/javascript

import { Rule } from 'eslint';
import { Mocha, getFullyQualifiedName, isIdentifier, isMethodInvocation } from '../helpers';
import * as estree from 'estree';
import { getDependencies } from '@sonar/jsts';

/**
* Cache for each dirname the dependencies of the package.json in this directory, empty set when no package.json.
*/
const dirCache: Map<string, Set<string>> = new Map();

/**
* Cache for the available dependencies by dirname.
*/
const cache: Map<string, Set<string>> = new Map();

export const rule: Rule.RuleModule = {
meta: {
messages: {
stable:
'Make your tests stable so that they pass on the first try, or remove the flaky ones.',
},
},
create(context: Rule.RuleContext) {
const describes: estree.Node[] = [];

const hasJest = hasJestDependency(context);

return {
/**
* We use the stack approach to check for Mocha retries inside describe blocks,
* and implicitly test cases.
*/
CallExpression(node: estree.CallExpression) {
if (hasJestRetry(context, node, hasJest)) {
report(context, node);
return;
}
if (Mocha.isDescribeCase(node)) {
describes.push(node);
return;
}
if (describes.length > 0) {
checkMochaRetries(context, node);
}
},
'CallExpression:exit': (node: estree.Node) => {
if (Mocha.isDescribeCase(node)) {
describes.pop();
}
},
'Program:exit': () => {
describes.length = 0;
},
};
},
};

function hasJestRetry(context: Rule.RuleContext, node: estree.CallExpression, hasJest: boolean) {
const callExpressionName = getFullyQualifiedName(context, node);
return (
callExpressionName === 'jest.retryTimes' ||
(hasJest && isMethodInvocation(node, 'jest', 'retryTimes', 1))
);
}

function hasJestDependency(context: Rule.RuleContext) {
const dependencies = getDependencies(context.filename, cache, dirCache);
return dependencies.has('jest');
}

/**
* Flag if `this.retries()`
*/
function checkMochaRetries(context: Rule.RuleContext, node: estree.CallExpression) {
const callee = node.callee;
if (
callee.type === 'MemberExpression' &&
callee.object.type === 'ThisExpression' &&
isIdentifier(callee.property, 'retries')
) {
report(context, node);
}
}

function report(context: Rule.RuleContext, node: estree.CallExpression) {
context.report({
messageId: 'stable',
node,
});
}
16 changes: 16 additions & 0 deletions packages/jsts/src/rules/helpers/mocha.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,23 @@ export namespace Mocha {
return null;
}

/**
* returns true if the node is a test case
*
* @param node
* @returns
*/
export function isTestCase(node: estree.Node): boolean {
return isTestConstruct(node, ['it', 'specify']);
}

/**
* returns true if the node is a describe block
*
* @param node
* @returns
*/
export function isDescribeCase(node: estree.Node): boolean {
return isTestConstruct(node, ['describe']);
}
}
2 changes: 2 additions & 0 deletions packages/jsts/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ import { rule as S6582 } from './S6582'; // sonar-prefer-optional-chain
import { rule as S6759 } from './S6759'; // sonar-prefer-read-only-props
import { rule as S6594 } from './S6594'; // sonar-prefer-regexp-exec
import { rule as S2077 } from './S2077'; // sql-queries
import { rule as S5973 } from './S5973'; // stable-tests
import { rule as S4829 } from './S4829'; // standard-input
import { rule as S6351 } from './S6351'; // stateful-regex
import { rule as S5739 } from './S5739'; // strict-transport-security
Expand Down Expand Up @@ -555,6 +556,7 @@ rules['sonar-prefer-optional-chain'] = S6582;
rules['sonar-prefer-read-only-props'] = S6759;
rules['sonar-prefer-regexp-exec'] = S6594;
rules['sql-queries'] = S2077;
rules['stable-tests'] = S5973;
rules['standard-input'] = S4829;
rules['stateful-regex'] = S6351;
rules['strict-transport-security'] = S5739;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ public static List<Class<? extends JavaScriptCheck>> getAllChecks() {
SonarNoMisleadingCharacterClassCheck.class,
SonarPreferOptionalChainCheck.class,
SqlQueriesCheck.class,
StableTestsCheck.class,
StandardInputCheck.class,
StatefulRegexCheck.class,
StrictModeCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2023 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.javascript.checks;

import org.sonar.check.Rule;
import org.sonar.plugins.javascript.api.JavaScriptRule;
import org.sonar.plugins.javascript.api.TestFileCheck;
import org.sonar.plugins.javascript.api.TypeScriptRule;

@TypeScriptRule
@JavaScriptRule
@Rule(key = "S5973")
public class StableTestsCheck extends TestFileCheck {

@Override
public String eslintKey() {
return "stable-tests";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<h2>Why is this an issue?</h2>
<p>Unstable / flaky tests are tests which sometimes pass and sometimes fail, without any code change. Obviously, they slow down developments when
developers have to rerun failed tests. However, the real problem is that you can’t completely trust these tests, they might fail for many different
reasons and you don’t know if any of them will happen in production.</p>
<p>Some tools, such as Jest, enable developers to automatically retry flaky tests. This might be acceptable as a temporary solution, but it should
eventually be fixed. The more flaky tests you add, the more chances there are for a bug to arrive in production.</p>
<p>This rule raises an issue when these functions are called with a value higher than <code>0</code>: * <code>jest.retry()</code> *
<code>this.retries()</code> inside a Mocha test case</p>
<h2>How to fix it</h2>
<p>Make your test stable so that it passes on the first try, or remove it.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre>
jest.retryTimes(3); // Noncompliant

describe('API.foo()', function() {
it('should return 5 when computing ...', function() {
doSomethingUnstable();
});
});
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li> Jest docs - <a href="https://jestjs.io/docs/jest-object#jestretrytimesnumretries-options">jest.retryTimes()</a> </li>
<li> Mocha docs - <a href="https://mochajs.org/#retry-tests">Retry tests</a> </li>
</ul>
<h3>Articles &amp; blog posts</h3>
<ul>
<li> Spotify Engineering - <a
href="https://engineering.atspotify.com/2019/11/18/test-flakiness-methods-for-identifying-and-dealing-with-flaky-tests/">Test Flakiness - Methods
for identifying and dealing with flaky tests</a> </li>
</ul>

Loading

0 comments on commit abf27ba

Please sign in to comment.