feat(cdk/testing): support querying for multiple TestHarness /… (#17658)

* feat(cdk/testing): support querying for multiple TestHarness / ComponentHarness at once in locatorFor

* fix lint and api goldens

* address feedback

* address feedback

* address comments

* address comments

Author: mmalerba

package.json

@@ -20,7 +20,7 @@
     "dev-app": "ibazel run //src/dev-app:devserver",
     "test": "bazel test //src/... --test_tag_filters=-e2e,-browser:firefox-local --build_tag_filters=-browser:firefox-local --build_tests_only",
     "test-firefox": "bazel test //src/... --test_tag_filters=-e2e,-browser:chromium-local --build_tag_filters=-browser:chromium-local --build_tests_only",
-    "lint": "gulp lint && yarn -s bazel:format-lint",
+    "lint": "yarn -s tslint && yarn -s bazel:format-lint && yarn -s ownerslint",
     "e2e": "bazel test //src/... --test_tag_filters=e2e",
     "deploy": "echo 'Not supported yet. Tracked with COMP-230'",
     "webdriver-manager": "webdriver-manager",

src/cdk/testing/component-harness.ts

@@ -12,10 +12,30 @@ import {
   ComponentHarnessConstructor,
   HarnessLoader,
   HarnessPredicate,
-  LocatorFactory
+  HarnessQuery,
+  LocatorFactory,
+  LocatorFnResult,
 } from './component-harness';
 import {TestElement} from './test-element';
 
+/** Parsed form of the queries passed to the `locatorFor*` methods. */
+type ParsedQueries<T extends ComponentHarness> = {
+  /** The full list of queries, in their original order. */
+  allQueries: (string | HarnessPredicate<T>)[],
+  /**
+   * A filtered view of `allQueries` containing only the queries that are looking for a
+   * `ComponentHarness`
+   */
+  harnessQueries: HarnessPredicate<T>[],
+  /**
+   * A filtered view of `allQueries` containing only the queries that are looking for a
+   * `TestElement`
+   */
+  elementQueries: string[],
+  /** The set of all `ComponentHarness` subclasses represented in the original query list. */
+  harnessTypes: Set<ComponentHarnessConstructor<T>>,
+};
+
 /**
  * Base harness environment class that can be extended to allow `ComponentHarness`es to be used in
  * different test environments (e.g. testbed, protractor, etc.). This class implements the
@@ -36,55 +56,29 @@ export abstract class HarnessEnvironment<E> implements HarnessLoader, LocatorFac
   }
 
   // Implemented as part of the `LocatorFactory` interface.
-  locatorFor(selector: string): AsyncFactoryFn<TestElement>;
-  locatorFor<T extends ComponentHarness>(
-      harnessType: ComponentHarnessConstructor<T> | HarnessPredicate<T>): AsyncFactoryFn<T>;
-  locatorFor<T extends ComponentHarness>(
-      arg: string | ComponentHarnessConstructor<T> | HarnessPredicate<T>) {
-    return async () => {
-      if (typeof arg === 'string') {
-        return this.createTestElement(await this._assertElementFound(arg));
-      } else {
-        return this._assertHarnessFound(arg);
-      }
-    };
+  locatorFor<T extends (HarnessQuery<any> | string)[]>(...queries: T):
+      AsyncFactoryFn<LocatorFnResult<T>> {
+    return () => _assertResultFound(
+        this._getAllHarnessesAndTestElements(queries),
+        _getDescriptionForLocatorForQueries(queries));
   }
 
   // Implemented as part of the `LocatorFactory` interface.
-  locatorForOptional(selector: string): AsyncFactoryFn<TestElement | null>;
-  locatorForOptional<T extends ComponentHarness>(
-      harnessType: ComponentHarnessConstructor<T> | HarnessPredicate<T>): AsyncFactoryFn<T | null>;
-  locatorForOptional<T extends ComponentHarness>(
-      arg: string | ComponentHarnessConstructor<T> | HarnessPredicate<T>) {
-    return async () => {
-      if (typeof arg === 'string') {
-        const element = (await this.getAllRawElements(arg))[0];
-        return element ? this.createTestElement(element) : null;
-      } else {
-        const candidates = await this._getAllHarnesses(arg);
-        return candidates[0] || null;
-      }
-    };
+  locatorForOptional<T extends (HarnessQuery<any> | string)[]>(...queries: T):
+      AsyncFactoryFn<LocatorFnResult<T> | null> {
+    return async () => (await this._getAllHarnessesAndTestElements(queries))[0] || null;
   }
 
   // Implemented as part of the `LocatorFactory` interface.
-  locatorForAll(selector: string): AsyncFactoryFn<TestElement[]>;
-  locatorForAll<T extends ComponentHarness>(
-      harnessType: ComponentHarnessConstructor<T> | HarnessPredicate<T>): AsyncFactoryFn<T[]>;
-  locatorForAll<T extends ComponentHarness>(
-      arg: string | ComponentHarnessConstructor<T> | HarnessPredicate<T>) {
-    return async () => {
-      if (typeof arg === 'string') {
-        return (await this.getAllRawElements(arg)).map(e => this.createTestElement(e));
-      } else {
-        return this._getAllHarnesses(arg);
-      }
-    };
+  locatorForAll<T extends (HarnessQuery<any> | string)[]>(...queries: T):
+      AsyncFactoryFn<LocatorFnResult<T>[]> {
+    return () => this._getAllHarnessesAndTestElements(queries);
   }
 
   // Implemented as part of the `LocatorFactory` interface.
   async harnessLoaderFor(selector: string): Promise<HarnessLoader> {
-    return this.createEnvironment(await this._assertElementFound(selector));
+    return this.createEnvironment(await _assertResultFound(this.getAllRawElements(selector),
+        [_getDescriptionForHarnessLoaderQuery(selector)]));
   }
 
   // Implemented as part of the `LocatorFactory` interface.
@@ -100,20 +94,19 @@ export abstract class HarnessEnvironment<E> implements HarnessLoader, LocatorFac
   }
 
   // Implemented as part of the `HarnessLoader` interface.
-  getHarness<T extends ComponentHarness>(
-      harnessType: ComponentHarnessConstructor<T> | HarnessPredicate<T>): Promise<T> {
-    return this.locatorFor(harnessType)();
+  getHarness<T extends ComponentHarness>(query: HarnessQuery<T>): Promise<T> {
+    return this.locatorFor(query)();
   }
 
   // Implemented as part of the `HarnessLoader` interface.
-  getAllHarnesses<T extends ComponentHarness>(
-      harnessType: ComponentHarnessConstructor<T> | HarnessPredicate<T>): Promise<T[]> {
-    return this.locatorForAll(harnessType)();
+  getAllHarnesses<T extends ComponentHarness>(query: HarnessQuery<T>): Promise<T[]> {
+    return this.locatorForAll(query)();
   }
 
   // Implemented as part of the `HarnessLoader` interface.
   async getChildLoader(selector: string): Promise<HarnessLoader> {
-    return this.createEnvironment(await this._assertElementFound(selector));
+    return this.createEnvironment(await _assertResultFound(this.getAllRawElements(selector),
+        [_getDescriptionForHarnessLoaderQuery(selector)]));
   }
 
   // Implemented as part of the `HarnessLoader` interface.
@@ -147,43 +140,147 @@ export abstract class HarnessEnvironment<E> implements HarnessLoader, LocatorFac
    */
   protected abstract getAllRawElements(selector: string): Promise<E[]>;
 
-  private async _getAllHarnesses<T extends ComponentHarness>(
-      harnessType: ComponentHarnessConstructor<T> | HarnessPredicate<T>): Promise<T[]> {
-    const harnessPredicate = harnessType instanceof HarnessPredicate ?
-        harnessType : new HarnessPredicate(harnessType, {});
-    const elements = await this.getAllRawElements(harnessPredicate.getSelector());
-    return harnessPredicate.filter(elements.map(
-        element => this.createComponentHarness(harnessPredicate.harnessType, element)));
+  /**
+   * Matches the given raw elements with the given list of element and harness queries to produce a
+   * list of matched harnesses and test elements.
+   */
+  private async _getAllHarnessesAndTestElements<T extends (HarnessQuery<any> | string)[]>(
+      queries: T): Promise<LocatorFnResult<T>[]> {
+    const {allQueries, harnessQueries, elementQueries, harnessTypes} = _parseQueries(queries);
+
+    // Combine all of the queries into one large comma-delimited selector and use it to get all raw
+    // elements matching any of the individual queries.
+    const rawElements = await this.getAllRawElements(
+        [...elementQueries, ...harnessQueries.map(predicate => predicate.getSelector())].join(','));
+
+    // If every query is searching for the same harness subclass, we know every result corresponds
+    // to an instance of that subclass. Likewise, if every query is for a `TestElement`, we know
+    // every result corresponds to a `TestElement`. Otherwise we need to verify which result was
+    // found by which selector so it can be matched to the appropriate instance.
+    const skipSelectorCheck = (elementQueries.length === 0 && harnessTypes.size === 1) ||
+        harnessQueries.length === 0;
+
+    const perElementMatches = await Promise.all(rawElements.map(async rawElement => {
+      const testElement = this.createTestElement(rawElement);
+      const allResultsForElement = await Promise.all(
+          // For each query, get `null` if it doesn't match, or a `TestElement` or
+          // `ComponentHarness` as appropriate if it does match. This gives us everything that
+          // matches the current raw element, but it may contain duplicate entries (e.g. multiple
+          // `TestElement` or multiple `ComponentHarness` of the same type.
+          allQueries.map(query =>
+              this._getQueryResultForElement(query, rawElement, testElement, skipSelectorCheck)));
+      return _removeDuplicateQueryResults(allResultsForElement);
+    }));
+    return ([] as any).concat(...perElementMatches);
   }
 
-  private async _assertElementFound(selector: string): Promise<E> {
-    const element = (await this.getAllRawElements(selector))[0];
-    if (!element) {
-      throw Error(`Expected to find element matching selector: "${selector}", but none was found`);
+  /**
+   * Check whether the given query matches the given element, if it does return the matched
+   * `TestElement` or `ComponentHarness`, if it does not, return null. In cases where the caller
+   * knows for sure that the query matches the element's selector, `skipSelectorCheck` can be used
+   * to skip verification and optimize performance.
+   */
+  private async _getQueryResultForElement<T extends ComponentHarness>(
+      query: string | HarnessPredicate<T>, rawElement: E, testElement: TestElement,
+      skipSelectorCheck: boolean = false): Promise<T | TestElement | null> {
+    if (typeof query === 'string') {
+      return ((skipSelectorCheck || await testElement.matchesSelector(query)) ? testElement : null);
     }
-    return element;
+    if (skipSelectorCheck || await testElement.matchesSelector(query.getSelector())) {
+      const harness = this.createComponentHarness(query.harnessType, rawElement);
+      return (await query.evaluate(harness)) ? harness : null;
+    }
+    return null;
   }
+}
+
+/**
+ * Parses a list of queries in the format accepted by the `locatorFor*` methods into an easier to
+ * work with format.
+ */
+function _parseQueries<T extends (HarnessQuery<any> | string)[]>(queries: T):
+    ParsedQueries<LocatorFnResult<T> & ComponentHarness> {
+  const allQueries = [];
+  const harnessQueries = [];
+  const elementQueries = [];
+  const harnessTypes =
+      new Set<ComponentHarnessConstructor<LocatorFnResult<T> & ComponentHarness>>();
 
-  private async _assertHarnessFound<T extends ComponentHarness>(
-      harnessType: ComponentHarnessConstructor<T> | HarnessPredicate<T>): Promise<T> {
-    const harness = (await this._getAllHarnesses(harnessType))[0];
-    if (!harness) {
-      throw _getErrorForMissingHarness(harnessType);
+  for (const query of queries) {
+    if (typeof query === 'string') {
+      allQueries.push(query);
+      elementQueries.push(query);
+    } else {
+      const predicate = query instanceof HarnessPredicate ? query : new HarnessPredicate(query, {});
+      allQueries.push(predicate);
+      harnessQueries.push(predicate);
+      harnessTypes.add(predicate.harnessType);
     }
-    return harness;
   }
+
+  return {allQueries, harnessQueries, elementQueries, harnessTypes};
+}
+
+/**
+ * Removes duplicate query results for a particular element. (e.g. multiple `TestElement`
+ * instances or multiple instances of the same `ComponentHarness` class.
+ */
+async function _removeDuplicateQueryResults<T extends (ComponentHarness | TestElement | null)[]>(
+    results: T): Promise<T> {
+  let testElementMatched = false;
+  let matchedHarnessTypes = new Set();
+  const dedupedMatches = [];
+  for (const result of results) {
+    if (!result) {
+      continue;
+    }
+    if (result instanceof ComponentHarness) {
+      if (!matchedHarnessTypes.has(result.constructor)) {
+        matchedHarnessTypes.add(result.constructor);
+        dedupedMatches.push(result);
+      }
+    } else if (!testElementMatched) {
+      testElementMatched = true;
+      dedupedMatches.push(result);
+    }
+  }
+  return dedupedMatches as T;
 }
 
-function _getErrorForMissingHarness<T extends ComponentHarness>(
-    harnessType: ComponentHarnessConstructor<T> | HarnessPredicate<T>): Error {
+/** Verifies that there is at least one result in an array. */
+async function _assertResultFound<T>(results: Promise<T[]>, queryDescriptions: string[]):
+    Promise<T> {
+  const result = (await results)[0];
+  if (result == undefined) {
+    throw Error(`Failed to find element matching one of the following queries:\n` +
+        queryDescriptions.map(desc => `(${desc})`).join(',\n'));
+  }
+  return result;
+}
+
+/** Gets a list of description strings from a list of queries. */
+function _getDescriptionForLocatorForQueries(queries: (string | HarnessQuery<any>)[]) {
+  return queries.map(query => typeof query === 'string' ?
+      _getDescriptionForTestElementQuery(query) : _getDescriptionForComponentHarnessQuery(query));
+}
+
+/** Gets a description string for a `ComponentHarness` query. */
+function _getDescriptionForComponentHarnessQuery(query: HarnessQuery<any>) {
   const harnessPredicate =
-      harnessType instanceof HarnessPredicate ? harnessType : new HarnessPredicate(harnessType, {});
+      query instanceof HarnessPredicate ? query : new HarnessPredicate(query, {});
   const {name, hostSelector} = harnessPredicate.harnessType;
-  let restrictions = harnessPredicate.getDescription();
-  let message = `Expected to find element for ${name} matching selector: "${hostSelector}"`;
-  if (restrictions) {
-    message += ` (with restrictions: ${restrictions})`;
-  }
-  message += ', but none was found';
-  return Error(message);
+  const description = `${name} with host element matching selector: "${hostSelector}"`;
+  const constraints = harnessPredicate.getDescription();
+  return description + (constraints ?
+      ` satisfying the constraints: ${harnessPredicate.getDescription()}` : '');
+}
+
+/** Gets a description string for a `TestElement` query. */
+function _getDescriptionForTestElementQuery(selector: string) {
+  return `TestElement for element matching selector: "${selector}"`;
+}
+
+/** Gets a description string for a `HarnessLoader` query. */
+function _getDescriptionForHarnessLoaderQuery(selector: string) {
+  return `HarnessLoader for element matching selector: "${selector}"`;
 }

src/cdk/testing/harness-environment.ts

@@ -10,13 +10,17 @@
  * Expects the asynchronous function to throw an error that matches
  * the specified expectation.
  */
-export async function expectAsyncError(fn: () => Promise<any>, expectation: RegExp) {
+export async function expectAsyncError(fn: () => Promise<any>, expectation: RegExp | string) {
   let error: string|null = null;
   try {
     await fn();
   } catch (e) {
     error = e.toString();
   }
   expect(error).not.toBe(null);
-  expect(error!).toMatch(expectation, 'Expected error to be thrown.');
+  if (expectation instanceof RegExp) {
+    expect(error!).toMatch(expectation, 'Expected error to be thrown.');
+  } else {
+    expect(error!).toBe(expectation, 'Expected error to be throw.');
+  }
 }

src/cdk/testing/private/expect-async-error.ts

@@ -37,6 +37,7 @@ ng_test_library(
         ":test_components",
         ":test_harnesses",
         "//src/cdk/testing",
+        "//src/cdk/testing/private",
         "//src/cdk/testing/testbed",
     ],
 )
@@ -47,6 +48,7 @@ ng_e2e_test_library(
     deps = [
         ":test_harnesses",
         "//src/cdk/testing",
+        "//src/cdk/testing/private",
         "//src/cdk/testing/protractor",
     ],
 )

src/cdk/testing/tests/BUILD.bazel

@@ -8,7 +8,7 @@
 
 import {ComponentHarness} from '../../component-harness';
 import {TestElement, TestKey} from '../../test-element';
-import {SubComponentHarness} from './sub-component-harness';
+import {SubComponentHarness, SubComponentSpecialHarness} from './sub-component-harness';
 
 export class WrongComponentHarness extends ComponentHarness {
   static readonly hostSelector = 'wrong-selector';
@@ -72,6 +72,15 @@ export class MainComponentHarness extends ComponentHarness {
   readonly directAncestorSelectorSubcomponent =
       this.locatorForAll(SubComponentHarness.with({ancestor: '.other >'}));
 
+  readonly subcomponentHarnessesAndElements =
+      this.locatorForAll('#counter', SubComponentHarness);
+  readonly subcomponentHarnessAndElementsRedundant =
+      this.locatorForAll(
+          SubComponentHarness.with({title: /test/}), 'test-sub', SubComponentHarness, 'test-sub');
+  readonly subcomponentAndSpecialHarnesses =
+      this.locatorForAll(SubComponentHarness, SubComponentSpecialHarness);
+  readonly missingElementsAndHarnesses =
+      this.locatorFor('.not-found', SubComponentHarness.with({title: /not found/}));
 
   private _testTools = this.locatorFor(SubComponentHarness);