WIP: address @jelbourn's comments

Author: mmalerba

src/cdk-experimental/testing/component-harness.ts

@@ -7,56 +7,59 @@
  */
 
 /**
- * Test Element interface
- * This is a wrapper of native element
+ * This acts as a common interface for DOM elements across both unit and e2e tests. It is the
+ * interface through which the ComponentHarness interacts with the component's DOM.
  */
 export interface TestElement {
+  /** Blur the element. */
   blur(): Promise<void>;
+  /** Clear the element's input (for input elements only). */
   clear(): Promise<void>;
+  /** Click the element. */
   click(): Promise<void>;
+  /** Focus the element. */
   focus(): Promise<void>;
+  /** Get the computed value of the given CSS property for the element. */
   getCssValue(property: string): Promise<string>;
+  /** Hovers the mouse over the element. */
   hover(): Promise<void>;
+  /**
+   * Sends the given string to the input as a series of key presses. Also fires input events
+   * and attempts to add the string to the Element's value.
+   */
   sendKeys(keys: string): Promise<void>;
+  /** Gets the text from the element. */
   text(): Promise<string>;
+  /**
+   * Gets the value for the given attribute from the element. If the attribute does not exist,
+   * falls back to reading the property.
+   */
   getAttribute(name: string): Promise<string|null>;
 }
 
-/**
- * Extra searching options used by searching functions
- *
- * @param allowNull Optional, whether the found element can be null. If
- * allowNull is set, the searching function will always try to fetch the element
- * at once. When the element cannot be found, the searching function should
- * return null if allowNull is set to true, throw an error if allowNull is set
- * to false. If allowNull is not set, the framework will choose the behaviors
- * that make more sense for each test type (e.g. for unit test, the framework
- * will make sure the element is not null; otherwise throw an error); however,
- * the internal behavior is not guaranteed and user should not rely on it. Note
- * that in most cases, you don't need to care about whether an element is
- * present when loading the element and don't need to set this parameter unless
- * you do want to check whether the element is present when calling the
- * searching function. e.g. you want to make sure some element is not there when
- * loading the element in order to check whether a "ngif" works well.
- *
- * @param global Optional. If global is set to true, the selector will match any
- *     element on the page and is not limited to the root of the harness. If
- * global is unset or set to false, the selector will only find elements under
- * the current root.
- */
-export interface Options {
+/** Options that can be specified when querying for an Element. */
+export interface QueryOptions {
+  /**
+   * Whether the found element can be null. If allowNull is set, the searching function will always
+   * try to fetch the element at once. When the element cannot be found, the searching function
+   * should return null if allowNull is set to true, throw an error if allowNull is set to false.
+   * If allowNull is not set, the framework will choose the behaviors that make more sense for each
+   * test type (e.g. for unit test, the framework will make sure the element is not null; otherwise
+   * throw an error); however, the internal behavior is not guaranteed and user should not rely on
+   * it. Note that in most cases, you don't need to care about whether an element is present when
+   * loading the element and don't need to set this parameter unless you do want to check whether
+   * the element is present when calling the searching function. e.g. you want to make sure some
+   * element is not there when loading the element in order to check whether a "ngif" works well.
+   */
   allowNull?: boolean;
+  /**
+   * If global is set to true, the selector will match any element on the page and is not limited to
+   * the root of the harness. If global is unset or set to false, the selector will only find
+   * elements under the current root.
+   */
   global?: boolean;
 }
 
-/**
- * Type narrowing of Options to allow the overloads of ComponentHarness.find to
- * return null only if allowNull is set to true.
- */
-interface OptionsWithAllowNullSet extends Options {
-  allowNull: true;
-}
-
 /**
  * Locator interface
  */
@@ -71,7 +74,7 @@ export interface Locator {
    * @param selector The CSS selector of the test elements.
    * @param options Optional, extra searching options
    */
-  querySelector(selector: string, options?: Options): Promise<TestElement|null>;
+  querySelector(selector: string, options?: QueryOptions): Promise<TestElement|null>;
 
   /**
    * Search all matched test elements under current root by CSS selector.
@@ -87,7 +90,7 @@ export interface Locator {
    */
   load<T extends ComponentHarness>(
     componentHarness: ComponentHarnessType<T>, root: string,
-    options?: Options): Promise<T|null>;
+    options?: QueryOptions): Promise<T|null>;
 
   /**
    * Load all Component Harnesses under current root.
@@ -127,7 +130,7 @@ export class ComponentHarness {
    * @param selector The CSS selector of the test element.
    * @param options Extra searching options
    */
-  protected find(selector: string, options: OptionsWithAllowNullSet):
+  protected find(selector: string, options: QueryOptions & {allowNull: true}):
     () => Promise<TestElement|null>;
 
   /**
@@ -136,7 +139,7 @@ export class ComponentHarness {
    * @param selector The CSS selector of the test element.
    * @param options Extra searching options
    */
-  protected find(selector: string, options: Options): () => Promise<TestElement>;
+  protected find(selector: string, options: QueryOptions): () => Promise<TestElement>;
 
   /**
    * Generate a function to find the first matched Component Harness.
@@ -155,7 +158,7 @@ export class ComponentHarness {
    */
   protected find<T extends ComponentHarness>(
     componentHarness: ComponentHarnessType<T>, root: string,
-    options: OptionsWithAllowNullSet): () => Promise<T|null>;
+    options: QueryOptions & {allowNull: true}): () => Promise<T|null>;
 
   /**
    * Generate a function to find the first matched Component Harness.
@@ -165,15 +168,15 @@ export class ComponentHarness {
    */
   protected find<T extends ComponentHarness>(
     componentHarness: ComponentHarnessType<T>, root: string,
-    options: Options): () => Promise<T>;
+    options: QueryOptions): () => Promise<T>;
 
   protected find<T extends ComponentHarness>(
     selectorOrComponentHarness: string|ComponentHarnessType<T>,
-    selectorOrOptions?: string|Options,
-    options?: Options): () => Promise<TestElement|T|null> {
+    selectorOrOptions?: string|QueryOptions,
+    options?: QueryOptions): () => Promise<TestElement|T|null> {
     if (typeof selectorOrComponentHarness === 'string') {
       const selector = selectorOrComponentHarness;
-      return () => this.locator.querySelector(selector, selectorOrOptions as Options);
+      return () => this.locator.querySelector(selector, selectorOrOptions as QueryOptions);
     } else {
       const componentHarness = selectorOrComponentHarness;
       const selector = selectorOrOptions as string;

src/cdk-experimental/testing/protractor.ts

@@ -15,7 +15,7 @@ import {
   ComponentHarness,
   ComponentHarnessType,
   Locator,
-  Options,
+  QueryOptions,
   TestElement
 } from './component-harness';
 
@@ -43,11 +43,11 @@ export async function load<T extends ComponentHarness>(
  */
 export async function load<T extends ComponentHarness>(
     componentHarness: ComponentHarnessType<T>, rootSelector?: string,
-    options?: Options): Promise<T|null>;
+    options?: QueryOptions): Promise<T|null>;
 
 export async function load<T extends ComponentHarness>(
     componentHarness: ComponentHarnessType<T>, rootSelector = 'body',
-    options?: Options): Promise<T|null> {
+    options?: QueryOptions): Promise<T|null> {
   const root = await getElement(rootSelector, undefined, options);
   return root && new componentHarness(new ProtractorLocator(root));
 }
@@ -74,25 +74,22 @@ class ProtractorLocator implements Locator {
     return this._root;
   }
 
-  async querySelector(selector: string, options?: Options): Promise<TestElement|null> {
+  async querySelector(selector: string, options?: QueryOptions): Promise<TestElement|null> {
     const finder = await getElement(selector, this._rootFinder, options);
     return finder && new ProtractorElement(finder);
   }
 
   async querySelectorAll(selector: string): Promise<TestElement[]> {
     const elementFinders = this._rootFinder.all(by.css(selector));
-    const res: TestElement[] = [];
-    await elementFinders.each(el => {
-      if (el) {
-        res.push(new ProtractorElement(el));
-      }
-    });
-    return res;
+    return elementFinders.reduce(
+      (result: TestElement[], el: ElementFinder) =>
+          el ? result.concat([new ProtractorElement(el)]) : result,
+      []);
   }
 
   async load<T extends ComponentHarness>(
       componentHarness: ComponentHarnessType<T>, selector: string,
-      options?: Options): Promise<T|null> {
+      options?: QueryOptions): Promise<T|null> {
     const root = await getElement(selector, this._rootFinder, options);
     return root && new componentHarness(new ProtractorLocator(root));
   }
@@ -101,14 +98,10 @@ class ProtractorLocator implements Locator {
       componentHarness: ComponentHarnessType<T>,
       rootSelector: string): Promise<T[]> {
     const roots = this._rootFinder.all(by.css(rootSelector));
-    const res: T[] = [];
-    await roots.each(el => {
-      if (el) {
-        const locator = new ProtractorLocator(el);
-        res.push(new componentHarness(locator));
-      }
-    });
-    return res;
+    return roots.reduce(
+        (result: T[], el: ElementFinder) =>
+            el ? result.concat(new componentHarness(new ProtractorLocator(el))) : result,
+        []);
   }
 }
 
@@ -163,7 +156,7 @@ class ProtractorElement implements TestElement {
  * search element globally. If options.global is set, root is ignored.
  * @param options Optional, extra searching options
  */
-async function getElement(selector: string, root?: ElementFinder, options?: Options):
+async function getElement(selector: string, root?: ElementFinder, options?: QueryOptions):
   Promise<ElementFinder|null> {
   const useGlobalRoot = options && !!options.global;
   const elem = root === undefined || useGlobalRoot ?

src/cdk-experimental/testing/testbed.ts

@@ -23,7 +23,7 @@ import {
   ComponentHarness,
   ComponentHarnessType,
   Locator,
-  Options,
+  QueryOptions,
   TestElement
 } from './component-harness';
 
@@ -68,7 +68,7 @@ export class UnitTestLocator implements Locator {
     return this._rootElement;
   }
 
-  async querySelector(selector: string, options?: Options): Promise<TestElement|null> {
+  async querySelector(selector: string, options?: QueryOptions): Promise<TestElement|null> {
     await this._stabilize();
     const e = getElement(selector, this._root, options);
     return e && new UnitTestElement(e, this._stabilize);
@@ -83,7 +83,7 @@ export class UnitTestLocator implements Locator {
 
   async load<T extends ComponentHarness>(
       componentHarness: ComponentHarnessType<T>, selector: string,
-      options?: Options): Promise<T|null> {
+      options?: QueryOptions): Promise<T|null> {
     await this._stabilize();
     const root = getElement(selector, this._root, options);
     return root && new componentHarness(new UnitTestLocator(root, this._stabilize));
@@ -187,7 +187,7 @@ class UnitTestElement implements TestElement {
  * to true, throw an error if Options.allowNull is set to false; otherwise,
  * return the element
  */
-function getElement(selector: string, root: Element, options?: Options): Element|null {
+function getElement(selector: string, root: Element, options?: QueryOptions): Element|null {
   const useGlobalRoot = options && !!options.global;
   const elem = (useGlobalRoot ? document : root).querySelector(selector);
   const allowNull = options !== undefined && options.allowNull !== undefined ?