diff --git a/.eslintrc.js b/.eslintrc.js index 28b337ddcd9c8..3e7ba874e7570 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -2703,8 +2703,9 @@ module.exports = { ], }, }, + // Custom rules for scout tests { - // Custom rules for scout tests + // Platform & Solutions files: [ 'src/platform/plugins/**/test/{scout,scout_*}/**/*.ts', 'x-pack/platform/**/plugins/**/test/{scout,scout_*}/**/*.ts', @@ -2714,13 +2715,26 @@ module.exports = { '@kbn/eslint/scout_no_describe_configure': 'error', '@kbn/eslint/scout_max_one_describe': 'error', '@kbn/eslint/scout_test_file_naming': 'error', - '@kbn/eslint/scout_require_api_client_in_api_test': 'error', '@kbn/eslint/scout_require_global_setup_hook_in_parallel_tests': 'error', '@kbn/eslint/scout_no_es_archiver_in_parallel_tests': 'error', '@kbn/eslint/scout_expect_import': 'error', '@kbn/eslint/require_include_in_check_a11y': 'warn', }, }, + { + // Platform & Solutions API Tests + files: [ + 'src/platform/plugins/**/test/{scout,scout_*}/api/**/*.ts', + 'x-pack/platform/**/plugins/**/test/{scout,scout_*}/api/**/*.ts', + 'x-pack/solutions/**/plugins/**/test/{scout,scout_*}/api/**/*.ts', + ], + rules: { + '@kbn/eslint/scout_require_api_client_in_api_test': [ + 'error', + { alternativeFixtures: ['esClient'] }, + ], + }, + }, { // Deployment-agnostic test files must use proper context and services files: [ diff --git a/packages/kbn-eslint-plugin-eslint/rules/scout_require_api_client_in_api_test.js b/packages/kbn-eslint-plugin-eslint/rules/scout_require_api_client_in_api_test.js index 3a73b6ec26dd5..c59b59cf838c9 100644 --- a/packages/kbn-eslint-plugin-eslint/rules/scout_require_api_client_in_api_test.js +++ b/packages/kbn-eslint-plugin-eslint/rules/scout_require_api_client_in_api_test.js @@ -11,26 +11,23 @@ /** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.CallExpression} CallExpression */ /** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.Identifier} Identifier */ -const ERROR_MSG = - 'The `apiClient` fixture should be used in `apiTest` to call an endpoint and later verify response code and body.'; - const traverse = require('eslint-traverse'); const isApiTestCall = (node) => node.callee.type === 'Identifier' && node.callee.name === 'apiTest'; -/** Get local param name for `apiClient` (supports `{ apiClient: alias }`). */ -const getApiClientLocalName = (fnNode) => { +/** Get local param name for a fixture (supports `{ fixtureName: alias }`). */ +const getFixtureLocalName = (fnNode, fixtureName) => { const firstParam = fnNode.params[0]; if (!firstParam || firstParam.type !== 'ObjectPattern') { - return 'apiClient'; + return fixtureName; } - const apiClientProp = firstParam.properties.find((prop) => { + const fixtureProp = firstParam.properties.find((prop) => { if (prop.type === 'RestElement') return false; - return prop.key && prop.key.name === 'apiClient'; + return prop.key && prop.key.name === fixtureName; }); - return apiClientProp?.value?.name || 'apiClient'; + return fixtureProp?.value?.name || fixtureName; }; /** Helper: Check if an identifier is used anywhere in a function body (simple recursion). */ @@ -65,18 +62,18 @@ const paramUsesIdentifierInBody = (node, paramName) => { }; /** - * Determine whether `fnNode` uses `apiClient`. + * Determine whether `fnNode` uses the given `fixtureName`. * * Strategy: Single-pass traversal that: - * 1. Collects variable aliases pointing to apiClient (e.g., `const client = apiClient`) + * 1. Collects variable aliases pointing to the fixture (e.g., `const client = apiClient`) * 2. Collects local function definitions and their parameter usage - * 3. Detects member access on apiClient or calls passing apiClient to functions that use it + * 3. Detects member access on the fixture or calls passing it to functions that use it * * We avoid the ESLint scope manager to maintain consistency with other rules in this package. */ -const functionUsesApiClient = (fnNode, context) => { - const apiClientName = getApiClientLocalName(fnNode); - const variableAliases = new Set([apiClientName]); +const functionUsesFixture = (fnNode, context, fixtureName) => { + const localName = getFixtureLocalName(fnNode, fixtureName); + const variableAliases = new Set([localName]); const localFnUsesParam = new Map(); let found = false; @@ -95,7 +92,7 @@ const functionUsesApiClient = (fnNode, context) => { // Pre-compute parameter usage for local function expressions if (node.init.type === 'FunctionExpression' || node.init.type === 'ArrowFunctionExpression') { const fnName = node.id.name; - const paramName = getApiClientLocalName(node.init); + const paramName = getFixtureLocalName(node.init, fixtureName); localFnUsesParam.set(fnName, paramUsesIdentifierInBody(node.init.body, paramName)); // Skip traversing into nested function bodies to avoid false positives return traverse.SKIP; @@ -105,13 +102,13 @@ const functionUsesApiClient = (fnNode, context) => { // Collect function declarations and pre-compute parameter usage if (node.type === 'FunctionDeclaration' && node.id?.type === 'Identifier') { const fnName = node.id.name; - const paramName = getApiClientLocalName(node); + const paramName = getFixtureLocalName(node, fixtureName); localFnUsesParam.set(fnName, paramUsesIdentifierInBody(node.body, paramName)); // Skip traversing into nested function bodies to avoid false positives return traverse.SKIP; } - // Detect member access on apiClient (e.g., `apiClient.get(...)`) + // Detect member access on the fixture (e.g., `apiClient.get(...)`) if ( node.type === 'MemberExpression' && node.object?.type === 'Identifier' && @@ -121,7 +118,7 @@ const functionUsesApiClient = (fnNode, context) => { return traverse.SKIP; } - // Detect calls with apiClient passed as argument + // Detect calls with the fixture passed as argument if (node.type === 'CallExpression' && node.arguments) { for (const arg of node.arguments) { if (arg.type === 'Identifier' && variableAliases.has(arg.name)) { @@ -140,7 +137,7 @@ const functionUsesApiClient = (fnNode, context) => { } } - // Also check if a local function is called that uses apiClient from closure (no args needed) + // Also check if a local function is called that uses the fixture from closure (no args needed) const callee = node.callee; if (callee.type === 'Identifier' && localFnUsesParam.has(callee.name)) { if (localFnUsesParam.get(callee.name)) { @@ -154,6 +151,18 @@ const functionUsesApiClient = (fnNode, context) => { return found; }; +/** Build a human-readable error message listing all accepted fixtures. */ +const buildErrorMsg = (acceptedFixtures) => { + if (acceptedFixtures.length === 1) { + return `The \`${acceptedFixtures[0]}\` fixture should be used in \`apiTest\` to call an endpoint and later verify response code and body.`; + } + const formatted = acceptedFixtures.map((f) => `\`${f}\``); + const last = formatted.pop(); + return `One of ${formatted.join( + ', ' + )} or ${last} fixtures should be used in \`apiTest\` to interact with an endpoint and later verify the response.`; +}; + /** @type {Rule} */ module.exports = { meta: { @@ -163,10 +172,27 @@ module.exports = { category: 'Best Practices', }, fixable: null, - schema: [], + schema: [ + { + type: 'object', + properties: { + alternativeFixtures: { + type: 'array', + items: { type: 'string' }, + uniqueItems: true, + }, + }, + additionalProperties: false, + }, + ], }, create(context) { + const options = context.options[0] || {}; + const alternativeFixtures = options.alternativeFixtures || []; + const acceptedFixtures = ['apiClient', ...alternativeFixtures]; + const errorMsg = buildErrorMsg(acceptedFixtures); + return { CallExpression(node) { if (!isApiTestCall(node)) return; @@ -193,10 +219,14 @@ module.exports = { return; } - if (!functionUsesApiClient(callbackArg, context)) { + const usesAcceptedFixture = acceptedFixtures.some((fixture) => + functionUsesFixture(callbackArg, context, fixture) + ); + + if (!usesAcceptedFixture) { context.report({ node, - message: ERROR_MSG, + message: errorMsg, }); } }, diff --git a/packages/kbn-eslint-plugin-eslint/rules/scout_require_api_client_in_api_test.test.js b/packages/kbn-eslint-plugin-eslint/rules/scout_require_api_client_in_api_test.test.js index 18332f9665563..fd435e7b60348 100644 --- a/packages/kbn-eslint-plugin-eslint/rules/scout_require_api_client_in_api_test.test.js +++ b/packages/kbn-eslint-plugin-eslint/rules/scout_require_api_client_in_api_test.test.js @@ -11,8 +11,10 @@ const { RuleTester } = require('eslint'); const rule = require('./scout_require_api_client_in_api_test'); const dedent = require('dedent'); -const ERROR_MSG = +const DEFAULT_ERROR_MSG = 'The `apiClient` fixture should be used in `apiTest` to call an endpoint and later verify response code and body.'; +const ALT_ERROR_MSG = + 'One of `apiClient` or `esClient` fixtures should be used in `apiTest` to interact with an endpoint and later verify the response.'; const ruleTester = new RuleTester({ parser: require.resolve('@typescript-eslint/parser'), parserOptions: { @@ -161,7 +163,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, { console.log(other); }); `, - errors: [{ message: ERROR_MSG }], + errors: [{ message: DEFAULT_ERROR_MSG }], }, // Nested apiTest inside apiTest.describe missing apiClient { @@ -170,7 +172,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, { apiTest('inner missing', () => {}); }); `, - errors: [{ message: ERROR_MSG }], + errors: [{ message: DEFAULT_ERROR_MSG }], }, // Nested blocks missing apiClient { @@ -181,7 +183,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, { } }); `, - errors: [{ message: ERROR_MSG }], + errors: [{ message: DEFAULT_ERROR_MSG }], }, // Nested helper function missing apiClient { @@ -193,7 +195,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, { helper(apiClient); }); `, - errors: [{ message: ERROR_MSG }], + errors: [{ message: DEFAULT_ERROR_MSG }], }, // Arrow function inside test { @@ -202,7 +204,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, { const callApi = () => apiClient.get('/bar'); }); `, - errors: [{ message: ERROR_MSG }], + errors: [{ message: DEFAULT_ERROR_MSG }], }, // External helper without apiClient usage { @@ -214,7 +216,7 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, { externalHelper(); }); `, - errors: [{ message: ERROR_MSG }], + errors: [{ message: DEFAULT_ERROR_MSG }], }, // Passing apiClient to a variable but never using it { @@ -224,7 +226,50 @@ ruleTester.run('@kbn/eslint/scout_require_api_client_in_api_test', rule, { fn(apiClient); }); `, - errors: [{ message: ERROR_MSG }], + errors: [{ message: DEFAULT_ERROR_MSG }], }, ], }); + +// --- Tests with alternativeFixtures option --- +const altOptions = [{ alternativeFixtures: ['esClient'] }]; + +ruleTester.run( + '@kbn/eslint/scout_require_api_client_in_api_test (with alternativeFixtures)', + rule, + { + valid: [ + // apiClient still accepted when alternativeFixtures is configured + { + code: dedent` + apiTest('uses apiClient', async ({ apiClient }) => { + await apiClient.get('/foo'); + }); + `, + options: altOptions, + }, + // Alternative fixture used instead of apiClient + { + code: dedent` + apiTest('uses esClient', async ({ esClient }) => { + await esClient.load('path/to/archive'); + }); + `, + options: altOptions, + }, + ], + + invalid: [ + // Neither apiClient nor alternative fixture used + { + code: dedent` + apiTest('uses neither', async () => { + console.log('no fixture'); + }); + `, + options: altOptions, + errors: [{ message: ALT_ERROR_MSG }], + }, + ], + } +);