revert(core): remove immutable rule flags

acot-a11y · May 5, 2022 · 7bd940b · 7bd940b
1 parent 753c1f1
commit 7bd940b
Show file tree

Hide file tree

Showing 13 changed files with 50 additions and 105 deletions.
diff --git a/docs/developer-guide/rule.md b/docs/developer-guide/rule.md
@@ -77,20 +77,6 @@ createRule<Options>({
   - returns: <[void]>
 - `options` <`T`> The option values passed to rule in configuration.
 
-#### `immutable`
-
-Set `true` if the rule does not change the page. Changes are, for example, changing the DOM or shifting the focus.
-
-```typescript
-createRule<Options>({
-  immutable: true,
-
-  /* ... */
-});
-```
-
-If the page doesn't change, audit runs in parallel to speed things up.
-
 #### `schema`
 
 Validate optional values in [JSON Schema](https://json-schema.org/) format.

diff --git a/packages/acot-preset-axe/src/create-rule.ts b/packages/acot-preset-axe/src/create-rule.ts
@@ -84,7 +84,6 @@ export type CreateRuleConfig = {
 
 export const createRule = (config: CreateRuleConfig): Rule<Options> => {
   return createAcotRule<Options>({
-    immutable: true,
     meta: {
       recommended: true,
     },

diff --git a/packages/acot-preset-wcag/src/rules/focusable-has-indicator.ts b/packages/acot-preset-wcag/src/rules/focusable-has-indicator.ts
@@ -17,7 +17,6 @@ const SELECTOR = [
 type Options = {};
 
 export default createRule<Options>({
-  immutable: true,
   meta: {
     help: 'https://www.w3.org/WAI/WCAG21/Understanding/focus-visible.html',
     recommended: true,

diff --git a/packages/acot-preset-wcag/src/rules/img-has-name.ts b/packages/acot-preset-wcag/src/rules/img-has-name.ts
@@ -3,7 +3,6 @@ import { createRule } from '@acot/core';
 type Options = {};
 
 export default createRule<Options>({
-  immutable: true,
   meta: {
     help: 'https://www.w3.org/WAI/WCAG21/Understanding/non-text-content.html',
     recommended: true,

diff --git a/packages/acot-preset-wcag/src/rules/interactive-has-enough-size.ts b/packages/acot-preset-wcag/src/rules/interactive-has-enough-size.ts
@@ -36,7 +36,6 @@ const SELECTOR = [
 type Options = {};
 
 export default createRule<Options>({
-  immutable: true,
   meta: {
     help: 'https://www.w3.org/WAI/WCAG21/Understanding/target-size.html',
     recommended: true,

diff --git a/packages/acot-preset-wcag/src/rules/interactive-has-name.ts b/packages/acot-preset-wcag/src/rules/interactive-has-name.ts
@@ -22,7 +22,6 @@ const SELECTOR = [
   .join('');
 
 export default createRule<Options>({
-  immutable: true,
   meta: {
     help: 'https://www.w3.org/WAI/WCAG21/Understanding/name-role-value.html',
     recommended: true,

diff --git a/packages/acot-preset-wcag/src/rules/interactive-supports-focus.ts b/packages/acot-preset-wcag/src/rules/interactive-supports-focus.ts
@@ -25,7 +25,6 @@ const SELECTOR = [
   .join('');
 
 export default createRule<Options>({
-  immutable: true,
   meta: {
     recommended: true,
   },

diff --git a/packages/acot-preset-wcag/src/rules/link-has-name.ts b/packages/acot-preset-wcag/src/rules/link-has-name.ts
@@ -3,7 +3,6 @@ import { createRule } from '@acot/core';
 type Options = {};
 
 export default createRule<Options>({
-  immutable: true,
   meta: {
     help: 'https://www.w3.org/WAI/WCAG21/Understanding/link-purpose-in-context.html',
     recommended: true,

diff --git a/packages/acot-preset-wcag/src/rules/page-has-title.ts b/packages/acot-preset-wcag/src/rules/page-has-title.ts
@@ -4,7 +4,6 @@ import { createRule } from '@acot/core';
 type Options = {};
 
 export default createRule<Options>({
-  immutable: true,
   meta: {
     help: 'https://www.w3.org/WAI/WCAG21/Understanding/page-titled.html',
     recommended: true,

diff --git a/packages/acot-preset-wcag/src/rules/page-has-valid-lang.ts b/packages/acot-preset-wcag/src/rules/page-has-valid-lang.ts
@@ -5,7 +5,6 @@ import tags from 'language-tags';
 type Options = {};
 
 export default createRule<Options>({
-  immutable: true,
   meta: {
     help: 'https://www.w3.org/WAI/WCAG21/Understanding/language-of-page.html',
     recommended: true,

diff --git a/packages/core/src/rule-context.ts b/packages/core/src/rule-context.ts
@@ -19,6 +19,9 @@ const rootDebug = require('debug')('acot');
 
 const writeFile = promisify(fs.writeFile);
 
+const errorMessage = (e: unknown) =>
+  e instanceof Error ? e.message : String(e);
+
 const buildFilename = (...args: string[]) => {
   return args
     .map((arg) => {
@@ -35,7 +38,7 @@ const element2selectorIfNeeded = async (node: ElementHandle | undefined) => {
   try {
     return await element2selector(node);
   } catch (e) {
-    debug('Element to Selector Error:', e);
+    debug('Element to Selector Error: %s', errorMessage(e));
     return null;
   }
 };
@@ -52,7 +55,7 @@ const screenshotIfNeeded = async (
     await node.screenshot({ path: to });
     return true;
   } catch (e) {
-    debug('Screenshot Error:', e);
+    debug('Screenshot Error: %s', errorMessage(e));
     return false;
   }
 };

diff --git a/packages/core/src/tester.ts b/packages/core/src/tester.ts
@@ -1,23 +1,22 @@
+import { createStat, createTestcaseResult } from '@acot/factory';
+import { validate } from '@acot/schema-validator';
 import type {
-  RuleId,
-  Rule,
+  CoreEventMap,
   NormalizedRuleConfig,
+  Rule,
+  RuleId,
+  Stat,
   Status,
   TestcaseResult,
   TestResult,
-  CoreEventMap,
-  Stat,
 } from '@acot/types';
-import { validate } from '@acot/schema-validator';
-import { createStat, createTestcaseResult } from '@acot/factory';
 import _ from 'lodash';
 import type { Page, Viewport } from 'puppeteer-core';
-import series from 'p-series';
-import type { BrowserPool } from './browser-pool';
-import type { RuleStore } from './rule-store';
-import { createRuleContext } from './rule-context';
 import type { Browser } from './browser';
+import type { BrowserPool } from './browser-pool';
 import { debug } from './logging';
+import { createRuleContext } from './rule-context';
+import type { RuleStore } from './rule-store';
 import { mark } from './timing';
 
 type TesterRuleOptions = NormalizedRuleConfig[RuleId];
@@ -62,52 +61,35 @@ export class Tester {
 
   public async test({ pool }: TesterContext): Promise<TestResult> {
     const { priority, url, onTestStart, onTestComplete } = this._config;
-    const { immutable, mutable } = this._prepare();
-    const ids = [...immutable, ...mutable].map(([id]) => id);
+    const rules = this._prepare();
+    const ids = rules.map(([id]) => id);
 
     await onTestStart(url, ids);
 
     const measure = mark();
 
     this._debug(`start ${ids.length} rules`);
 
-    await Promise.allSettled([
-      immutable.length > 0
-        ? pool.execute(priority, async (browser) => {
-            const page = await this._createPage(browser);
-
-            try {
-              await series(
-                immutable.map((args) => async () => {
-                  await this._execute(browser, page, args);
-                  await this._cleanupPage(page);
-                }),
-              );
-            } catch (e) {
-              this._debug(e);
-            } finally {
-              await page.close();
-            }
-          })
-        : Promise.resolve(),
-      ...mutable.map(async (loop) => {
-        await pool.execute(
+    await Promise.allSettled(
+      rules.map((loop) =>
+        pool.execute(
           priority,
           async (browser, args) => {
-            const page = await this._createPage(browser);
+            let page: Page | null = null;
 
             try {
+              page = await this._createPage(browser);
               await this._execute(browser, page, args);
             } catch (e) {
               this._debug(e);
             } finally {
-              await page.close();
+              await page?.close();
             }
           },
           loop,
-        );
-      }),
-    ]);
+        ),
+      ),
+    );
 
     const rulesAndStat = this._results.reduce<
       Pick<TestResult, keyof Stat | 'rules'>
@@ -168,11 +150,8 @@ export class Tester {
     return result;
   }
 
-  private _prepare(): {
-    immutable: TesterRuleGroup[];
-    mutable: TesterRuleGroup[];
-  } {
-    return Object.entries(this._config.rules).reduce(
+  private _prepare(): TesterRuleGroup[] {
+    return Object.entries(this._config.rules).reduce<TesterRuleGroup[]>(
       (acc, [id, options]) => {
         const rule = this._config.store.get(id);
         if (rule == null) {
@@ -201,34 +180,25 @@ export class Tester {
           } catch (e) {
             debug('validation error:', e);
 
+            const msg = e instanceof Error ? e.message : String(e);
+
             this._results.push(
               createTestcaseResult({
                 status: 'error',
                 rule: id,
-                message: `${id} validation error: ${
-                  e instanceof Error ? e.message : String(e)
-                }`,
+                message: `${id} validation error: ${msg}`,
               }),
             );
 
             return acc;
           }
         }
 
-        const group: TesterRuleGroup = [id, rule, opts];
-
-        if (rule.immutable) {
-          acc.immutable.push(group);
-        } else {
-          acc.mutable.push(group);
-        }
+        acc.push([id, rule, opts]);
 
         return acc;
       },
-      {
-        immutable: [] as TesterRuleGroup[],
-        mutable: [] as TesterRuleGroup[],
-      },
+      [],
     );
   }
 
@@ -250,8 +220,24 @@ export class Tester {
     return page;
   }
 
-  private async _cleanupPage(page: Page): Promise<void> {
-    await page.focus('body');
+  private async _waitForReady(page: Page): Promise<void> {
+    const { url, readyTimeout: timeout } = this._config;
+
+    try {
+      await Promise.all([
+        page.goto(url, {
+          waitUntil: 'networkidle2',
+          timeout,
+        }),
+        page.waitForNavigation({
+          waitUntil: 'domcontentloaded',
+          timeout,
+        }),
+      ]);
+    } catch (e) {
+      this._debug(e);
+      throw e;
+    }
   }
 
   private async _execute(
@@ -316,26 +302,6 @@ export class Tester {
     this._results.push(...results);
   }
 
-  private async _waitForReady(page: Page): Promise<void> {
-    const { url, readyTimeout: timeout } = this._config;
-
-    try {
-      await Promise.all([
-        page.goto(url, {
-          waitUntil: 'networkidle2',
-          timeout,
-        }),
-        page.waitForNavigation({
-          waitUntil: 'domcontentloaded',
-          timeout,
-        }),
-      ]);
-    } catch (e) {
-      this._debug(e);
-      throw e;
-    }
-  }
-
   private _debug(...args: any[]) {
     const { url, priority } = this._config;
 

diff --git a/packages/types/src/rule.ts b/packages/types/src/rule.ts
@@ -24,7 +24,6 @@ export type RuleMeta = {
 
 export type Rule<T extends RuleOptions = RuleOptions> = {
   schema?: Schema;
-  immutable?: boolean;
   meta?: RuleMeta;
   test: (context: RuleContext<T>) => Promise<void>;
 };