Skip to content

Commit

Permalink
fix(valid-title): Fix more false positives and reduce future chance o…
Browse files Browse the repository at this point in the history
…f error
  • Loading branch information
mskelton committed Feb 27, 2024
1 parent 33b2905 commit 3e99e55
Show file tree
Hide file tree
Showing 7 changed files with 441 additions and 79 deletions.
18 changes: 3 additions & 15 deletions src/rules/no-conditional-in-test.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,14 @@
import { Rule } from 'eslint';
import { findParent, getStringValue } from '../utils/ast';
import { parseFnCall } from '../utils/parseFnCall';
import { findParent } from '../utils/ast';
import { isTypeOfFnCall } from '../utils/parseFnCall';

export default {
create(context) {
function checkConditional(node: Rule.Node & Rule.NodeParentExtension) {
const call = findParent(node, 'CallExpression');
if (!call) return;

const fnCall = parseFnCall(context, call);

// Allow conditional logic in `test.skip()` calls inside of tests
// https://playwright.dev/docs/api/class-test#test-skip-3
if (
fnCall?.type === 'test' &&
fnCall.members.some((member) => getStringValue(member) === 'skip') &&
call.arguments[0]?.type === 'LogicalExpression'
) {
return;
}

if (fnCall?.type === 'test' || fnCall?.type === 'step') {
if (isTypeOfFnCall(context, call, ['test', 'step'])) {
context.report({ messageId: 'conditionalInTest', node });
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/rules/no-skipped-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ export default {
const options = context.options[0] || {};
const allowConditional = !!options.allowConditional;

const call = parseFnCall(context, node);
const call = parseFnCall(context, node, {
includeConfigStatements: true,
});
if (call?.type !== 'test' && call?.type !== 'describe') {
return;
}
Expand Down
12 changes: 12 additions & 0 deletions src/rules/require-top-level-describe.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,18 @@ runRuleTester('require-top-level-describe', rule, {
valid: [
'foo()',
'test.info()',
'test.use({ locale: "en-US" })',
// 'test.only("that all is as it should be", () => {});',
// 'test.skip("that all is as it should be", () => {});',
// 'test.skip(({ browserName }) => browserName === "Chrome");',
// 'test.skip();',
// 'test.skip(true);',
// 'test.skip(browserName === "Chrome", "This feature is skipped on Chrome")',
// 'test.slow("that all is as it should be", () => {});',
// 'test.slow(({ browserName }) => browserName === "Chrome");',
// 'test.slow();',
// 'test.slow(browserName === "webkit", "This feature is slow on Mac")',
'test.describe.configure({ mode: "parallel" })',
'test.describe("suite", () => { test("foo") });',
'test.describe.only("suite", () => { test.beforeAll("my beforeAll") });',
'test.describe.parallel("suite", () => { test.beforeEach("my beforeAll") });',
Expand Down
4 changes: 4 additions & 0 deletions src/rules/valid-title.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,12 @@ runRuleTester('valid-title', rule, {
'test.skip("that all is as it should be", () => {});',
'test.skip(({ browserName }) => browserName === "Chrome");',
'test.skip();',
'test.skip(true);',
'test.slow(true, "Always");',
'test.skip(browserName === "Chrome", "This feature is skipped on Chrome")',
'test.slow("that all is as it should be", () => {});',
'test.slow(true);',
'test.slow(true, "Always");',
'test.slow(({ browserName }) => browserName === "Chrome");',
'test.slow();',
'test.slow(browserName === "webkit", "This feature is slow on Mac")',
Expand Down
9 changes: 1 addition & 8 deletions src/rules/valid-title.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,8 @@ export default {
return;
}

// Ignore statements such as `test.slow()`, `test.describe.configure()`, etc.
if (node.arguments.length < 2) {
return;
}

const [argument] = node.arguments;
if (!argument) {
return;
}
if (!argument) return;

if (!isStringNode(argument)) {
if (
Expand Down
Loading

0 comments on commit 3e99e55

Please sign in to comment.