From 0520d53536af411d66ce2ce0dd478365e67adbac Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Mon, 31 Oct 2022 05:03:38 -0700 Subject: [PATCH] feat(utils): add `RuleTester` API for top-level dependency constraints (#5896) * feat(utils): add `RuleTester` API for top-level dependency constraints * Apply suggestions from code review Co-authored-by: Josh Goldberg * address comments * oops Co-authored-by: Josh Goldberg --- .../eslint-utils/rule-tester/RuleTester.ts | 89 +++++++++++++++--- packages/utils/src/ts-eslint/RuleTester.ts | 21 ++--- .../rule-tester/RuleTester.test.ts | 91 ++++++++++++++++++- 3 files changed, 175 insertions(+), 26 deletions(-) diff --git a/packages/utils/src/eslint-utils/rule-tester/RuleTester.ts b/packages/utils/src/eslint-utils/rule-tester/RuleTester.ts index e81d23d0206..54a645ccf25 100644 --- a/packages/utils/src/eslint-utils/rule-tester/RuleTester.ts +++ b/packages/utils/src/eslint-utils/rule-tester/RuleTester.ts @@ -1,9 +1,13 @@ import type * as TSESLintParserType from '@typescript-eslint/parser'; +import assert from 'assert'; import { version as eslintVersion } from 'eslint/package.json'; import * as path from 'path'; import * as semver from 'semver'; -import * as TSESLint from '../../ts-eslint'; +import type { ParserOptions } from '../../ts-eslint/ParserOptions'; +import type { RuleModule } from '../../ts-eslint/Rule'; +import type { RuleTesterTestFrameworkFunction } from '../../ts-eslint/RuleTester'; +import * as BaseRuleTester from '../../ts-eslint/RuleTester'; import { deepMerge } from '../deepMerge'; import type { DependencyConstraint } from './dependencyConstraints'; import { satisfiesAllDependencyConstraints } from './dependencyConstraints'; @@ -11,18 +15,28 @@ import { satisfiesAllDependencyConstraints } from './dependencyConstraints'; const TS_ESLINT_PARSER = '@typescript-eslint/parser'; const ERROR_MESSAGE = `Do not set the parser at the test level unless you want to use a parser other than ${TS_ESLINT_PARSER}`; -type RuleTesterConfig = Omit & { +type RuleTesterConfig = Omit & { parser: typeof TS_ESLINT_PARSER; + /** + * Constraints that must pass in the current environment for any tests to run + */ + dependencyConstraints?: DependencyConstraint; }; interface InvalidTestCase< TMessageIds extends string, TOptions extends Readonly, -> extends TSESLint.InvalidTestCase { +> extends BaseRuleTester.InvalidTestCase { + /** + * Constraints that must pass in the current environment for the test to run + */ dependencyConstraints?: DependencyConstraint; } interface ValidTestCase> - extends TSESLint.ValidTestCase { + extends BaseRuleTester.ValidTestCase { + /** + * Constraints that must pass in the current environment for the test to run + */ dependencyConstraints?: DependencyConstraint; } interface RunTests< @@ -36,24 +50,42 @@ interface RunTests< type AfterAll = (fn: () => void) => void; -class RuleTester extends TSESLint.RuleTester { +function isDescribeWithSkip( + value: unknown, +): value is RuleTesterTestFrameworkFunction & { + skip: RuleTesterTestFrameworkFunction; +} { + return ( + typeof value === 'object' && + value != null && + 'skip' in value && + typeof (value as Record).skip === 'function' + ); +} + +class RuleTester extends BaseRuleTester.RuleTester { readonly #baseOptions: RuleTesterConfig; - static #afterAll: AfterAll; + static #afterAll: AfterAll | undefined; /** * If you supply a value to this property, the rule tester will call this instead of using the version defined on * the global namespace. */ static get afterAll(): AfterAll { return ( - this.#afterAll || + this.#afterAll ?? (typeof afterAll === 'function' ? afterAll : (): void => {}) ); } - static set afterAll(value) { + static set afterAll(value: AfterAll | undefined) { this.#afterAll = value; } + private get staticThis(): typeof RuleTester { + // the cast here is due to https://github.com/microsoft/TypeScript/issues/3841 + return this.constructor as typeof RuleTester; + } + constructor(baseOptions: RuleTesterConfig) { super({ ...baseOptions, @@ -73,8 +105,7 @@ class RuleTester extends TSESLint.RuleTester { // make sure that the parser doesn't hold onto file handles between tests // on linux (i.e. our CI env), there can be very a limited number of watch handles available - // the cast here is due to https://github.com/microsoft/TypeScript/issues/3841 - (this.constructor as typeof RuleTester).afterAll(() => { + this.staticThis.afterAll(() => { try { // instead of creating a hard dependency, just use a soft require // a bit weird, but if they're using this tooling, it'll be installed @@ -85,11 +116,11 @@ class RuleTester extends TSESLint.RuleTester { } }); } - private getFilename(testOptions?: TSESLint.ParserOptions): string { + private getFilename(testOptions?: ParserOptions): string { const resolvedOptions = deepMerge( this.#baseOptions.parserOptions, testOptions, - ) as TSESLint.ParserOptions; + ) as ParserOptions; const filename = `file.ts${resolvedOptions.ecmaFeatures?.jsx ? 'x' : ''}`; if (resolvedOptions.project) { return path.join( @@ -107,9 +138,41 @@ class RuleTester extends TSESLint.RuleTester { // This is a lot more explicit run>( name: string, - rule: TSESLint.RuleModule, + rule: RuleModule, testsReadonly: RunTests, ): void { + if ( + this.#baseOptions.dependencyConstraints && + !satisfiesAllDependencyConstraints( + this.#baseOptions.dependencyConstraints, + ) + ) { + if (isDescribeWithSkip(this.staticThis.describe)) { + // for frameworks like mocha or jest that have a "skip" version of their function + // we can provide a nice skipped test! + this.staticThis.describe.skip(name, () => { + this.staticThis.it( + 'All tests skipped due to unsatisfied constructor dependency constraints', + () => {}, + ); + }); + } else { + // otherwise just declare an empty test + this.staticThis.describe(name, () => { + this.staticThis.it( + 'All tests skipped due to unsatisfied constructor dependency constraints', + () => { + // some frameworks error if there are no assertions + assert.equal(true, true); + }, + ); + }); + } + + // don't run any tests because we don't match the base constraint + return; + } + const tests = { // standardize the valid tests as objects valid: testsReadonly.valid.map(test => { diff --git a/packages/utils/src/ts-eslint/RuleTester.ts b/packages/utils/src/ts-eslint/RuleTester.ts index 7002fc538cd..6c0b98b795f 100644 --- a/packages/utils/src/ts-eslint/RuleTester.ts +++ b/packages/utils/src/ts-eslint/RuleTester.ts @@ -125,6 +125,10 @@ interface TestCaseError { // readonly message?: string | RegExp; } +/** + * @param text a string describing the rule + * @param callback the test callback + */ type RuleTesterTestFrameworkFunction = ( text: string, callback: () => void, @@ -166,31 +170,26 @@ declare class RuleTesterBase { /** * If you supply a value to this property, the rule tester will call this instead of using the version defined on * the global namespace. - * @param text a string describing the rule - * @param callback the test callback */ - static describe?: RuleTesterTestFrameworkFunction; + static get describe(): RuleTesterTestFrameworkFunction; + static set describe(value: RuleTesterTestFrameworkFunction | undefined); /** * If you supply a value to this property, the rule tester will call this instead of using the version defined on * the global namespace. - * @param text a string describing the test case - * @param callback the test callback */ - static it?: RuleTesterTestFrameworkFunction; + static get it(): RuleTesterTestFrameworkFunction; + static set it(value: RuleTesterTestFrameworkFunction | undefined); /** * If you supply a value to this property, the rule tester will call this instead of using the version defined on * the global namespace. - * @param text a string describing the test case - * @param callback the test callback */ - static itOnly?: RuleTesterTestFrameworkFunction; + static get itOnly(): RuleTesterTestFrameworkFunction; + static set itOnly(value: RuleTesterTestFrameworkFunction | undefined); /** * Define a rule for one particular run of tests. - * @param name The name of the rule to define. - * @param rule The rule definition. */ defineRule>( name: string, diff --git a/packages/utils/tests/eslint-utils/rule-tester/RuleTester.test.ts b/packages/utils/tests/eslint-utils/rule-tester/RuleTester.test.ts index cab7666196a..2e620332942 100644 --- a/packages/utils/tests/eslint-utils/rule-tester/RuleTester.test.ts +++ b/packages/utils/tests/eslint-utils/rule-tester/RuleTester.test.ts @@ -73,8 +73,8 @@ RuleTester.itOnly = jest.fn(); /* eslint-enable jest/prefer-spy-on */ const mockedAfterAll = jest.mocked(RuleTester.afterAll); -const _mockedDescribe = jest.mocked(RuleTester.describe); -const _mockedIt = jest.mocked(RuleTester.it); +const mockedDescribe = jest.mocked(RuleTester.describe); +const mockedIt = jest.mocked(RuleTester.it); const _mockedItOnly = jest.mocked(RuleTester.itOnly); const runSpy = jest.spyOn(BaseRuleTester.prototype, 'run'); const mockedParserClearCaches = jest.mocked(parser.clearCaches); @@ -715,5 +715,92 @@ describe('RuleTester', () => { } `); }); + + describe('constructor constraints', () => { + it('skips all tests if a constructor constraint is not satisifed', () => { + const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + dependencyConstraints: { + 'totally-real-dependency': '999', + }, + }); + + ruleTester.run('my-rule', NOOP_RULE, { + invalid: [ + { + code: 'failing - major', + errors: [], + }, + ], + valid: [ + { + code: 'passing - major', + }, + ], + }); + + // trigger the describe block + expect(mockedDescribe.mock.calls.length).toBeGreaterThanOrEqual(1); + mockedDescribe.mock.lastCall?.[1](); + expect(mockedDescribe.mock.calls).toMatchInlineSnapshot(` + [ + [ + "my-rule", + [Function], + ], + ] + `); + expect(mockedIt.mock.lastCall).toMatchInlineSnapshot(` + [ + "All tests skipped due to unsatisfied constructor dependency constraints", + [Function], + ] + `); + }); + + it('does not skip all tests if a constructor constraint is satisifed', () => { + const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + dependencyConstraints: { + 'totally-real-dependency': '10', + }, + }); + + ruleTester.run('my-rule', NOOP_RULE, { + invalid: [ + { + code: 'valid', + errors: [], + }, + ], + valid: [ + { + code: 'valid', + }, + ], + }); + + // trigger the describe block + expect(mockedDescribe.mock.calls.length).toBeGreaterThanOrEqual(1); + mockedDescribe.mock.lastCall?.[1](); + expect(mockedDescribe.mock.calls).toMatchInlineSnapshot(` + [ + [ + "my-rule", + [Function], + ], + [ + "valid", + [Function], + ], + [ + "invalid", + [Function], + ], + ] + `); + // expect(mockedIt.mock.lastCall).toMatchInlineSnapshot(`undefined`); + }); + }); }); });