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 <git@joshuakgoldberg.com> * address comments * oops Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>
typescript-eslint · Oct 31, 2022 · 0520d53 · 0520d53
1 parent 1f14c03
commit 0520d53
Show file tree

Hide file tree

Showing 3 changed files with 175 additions and 26 deletions.
diff --git a/packages/utils/src/eslint-utils/rule-tester/RuleTester.ts b/packages/utils/src/eslint-utils/rule-tester/RuleTester.ts
@@ -1,28 +1,42 @@
 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';
 
 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<TSESLint.RuleTesterConfig, 'parser'> & {
+type RuleTesterConfig = Omit<BaseRuleTester.RuleTesterConfig, 'parser'> & {
   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<unknown[]>,
-> extends TSESLint.InvalidTestCase<TMessageIds, TOptions> {
+> extends BaseRuleTester.InvalidTestCase<TMessageIds, TOptions> {
+  /**
+   * Constraints that must pass in the current environment for the test to run
+   */
   dependencyConstraints?: DependencyConstraint;
 }
 interface ValidTestCase<TOptions extends Readonly<unknown[]>>
-  extends TSESLint.ValidTestCase<TOptions> {
+  extends BaseRuleTester.ValidTestCase<TOptions> {
+  /**
+   * 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<string, unknown>).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<TMessageIds extends string, TOptions extends Readonly<unknown[]>>(
     name: string,
-    rule: TSESLint.RuleModule<TMessageIds, TOptions>,
+    rule: RuleModule<TMessageIds, TOptions>,
     testsReadonly: RunTests<TMessageIds, TOptions>,
   ): 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
@@ -125,6 +125,10 @@ interface TestCaseError<TMessageIds extends string> {
   // 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<TMessageIds extends string, TOptions extends Readonly<unknown[]>>(
     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
@@ -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`);
+      });
+    });
   });
 });