address crisbeto@'s feedback

Author: mmalerba

.github/CODEOWNERS

@@ -178,6 +178,7 @@
 /src/e2e-app/button-toggle/**                      @jelbourn
 /src/e2e-app/card/**                               @jelbourn
 /src/e2e-app/checkbox/**                           @jelbourn @devversion
+/src/e2e-app/component-harness/**                  @mmalerba
 /src/e2e-app/dialog/**                             @jelbourn @crisbeto
 /src/e2e-app/e2e-app/**                            @jelbourn
 /src/e2e-app/example-viewer/**                     @andrewseguin

src/cdk-experimental/testing/BUILD.bazel

@@ -11,6 +11,7 @@ ng_module(
   deps = [
     "@npm//@angular/core",
     "@npm//protractor",
+    "//src/cdk/testing",
   ],
 )
 

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

@@ -68,21 +68,21 @@ export interface Locator {
 
   /**
    * Search the first matched test element.
-   * @param css Selector of the test elements.
+   * @param selector The CSS selector of the test elements.
    * @param options Optional, extra searching options
    */
-  find(css: string, options?: Options): Promise<TestElement|null>;
+  querySelector(selector: string, options?: Options): Promise<TestElement|null>;
 
   /**
-   * Search all matched test elements under current root by css selector.
-   * @param css Selector of the test elements.
+   * Search all matched test elements under current root by CSS selector.
+   * @param selector The CSS selector of the test elements.
    */
-  findAll(css: string): Promise<TestElement[]>;
+  querySelectorAll(selector: string): Promise<TestElement[]>;
 
   /**
    * Load the first matched Component Harness.
    * @param componentHarness Type of user customized harness.
-   * @param root Css root selector of the new component harness.
+   * @param root CSS root selector of the new component harness.
    * @param options Optional, extra searching options
    */
   load<T extends ComponentHarness>(
@@ -92,7 +92,7 @@ export interface Locator {
   /**
    * Load all Component Harnesses under current root.
    * @param componentHarness Type of user customized harness.
-   * @param root Css root selector of the new component harnesses.
+   * @param root CSS root selector of the new component harnesses.
    */
   loadAll<T extends ComponentHarness>(
     componentHarness: ComponentHarnessType<T>, root: string): Promise<T[]>;
@@ -115,33 +115,33 @@ export class ComponentHarness {
   }
 
   /**
-   * Generate a function to find the first matched test element by css
+   * Generate a function to find the first matched test element by CSS
    * selector.
-   * @param css Css selector of the test element.
+   * @param selector The CSS selector of the test element.
    */
-  protected find(css: string): () => Promise<TestElement>;
+  protected find(selector: string): () => Promise<TestElement>;
 
   /**
-   * Generate a function to find the first matched test element by css
+   * Generate a function to find the first matched test element by CSS
    * selector.
-   * @param css Css selector of the test element.
+   * @param selector The CSS selector of the test element.
    * @param options Extra searching options
    */
-  protected find(css: string, options: OptionsWithAllowNullSet):
+  protected find(selector: string, options: OptionsWithAllowNullSet):
     () => Promise<TestElement|null>;
 
   /**
-   * Generate a function to find the first matched test element by css
+   * Generate a function to find the first matched test element by CSS
    * selector.
-   * @param css Css selector of the test element.
+   * @param selector The CSS selector of the test element.
    * @param options Extra searching options
    */
-  protected find(css: string, options: Options): () => Promise<TestElement>;
+  protected find(selector: string, options: Options): () => Promise<TestElement>;
 
   /**
    * Generate a function to find the first matched Component Harness.
    * @param componentHarness Type of user customized harness.
-   * @param root Css root selector of the new component harness.
+   * @param root CSS root selector of the new component harness.
    */
   protected find<T extends ComponentHarness>(
     componentHarness: ComponentHarnessType<T>,
@@ -150,7 +150,7 @@ export class ComponentHarness {
   /**
    * Generate a function to find the first matched Component Harness.
    * @param componentHarness Type of user customized harness.
-   * @param root Css root selector of the new component harness.
+   * @param root CSS root selector of the new component harness.
    * @param options Extra searching options
    */
   protected find<T extends ComponentHarness>(
@@ -160,53 +160,53 @@ export class ComponentHarness {
   /**
    * Generate a function to find the first matched Component Harness.
    * @param componentHarness Type of user customized harness.
-   * @param root Css root selector of the new component harness.
+   * @param root CSS root selector of the new component harness.
    * @param options Extra searching options
    */
   protected find<T extends ComponentHarness>(
     componentHarness: ComponentHarnessType<T>, root: string,
     options: Options): () => Promise<T>;
 
   protected find<T extends ComponentHarness>(
-    cssOrComponentHarness: string|ComponentHarnessType<T>,
-    cssOrOptions?: string|Options,
+    selectorOrComponentHarness: string|ComponentHarnessType<T>,
+    selectorOrOptions?: string|Options,
     options?: Options): () => Promise<TestElement|T|null> {
-    if (typeof cssOrComponentHarness === 'string') {
-      const css = cssOrComponentHarness;
-      return () => this.locator.find(css, cssOrOptions as Options);
+    if (typeof selectorOrComponentHarness === 'string') {
+      const selector = selectorOrComponentHarness;
+      return () => this.locator.querySelector(selector, selectorOrOptions as Options);
     } else {
-      const componentHarness = cssOrComponentHarness;
-      const css = cssOrOptions as string;
-      return () => this.locator.load(componentHarness, css, options);
+      const componentHarness = selectorOrComponentHarness;
+      const selector = selectorOrOptions as string;
+      return () => this.locator.load(componentHarness, selector, options);
     }
   }
 
   /**
-   * Generate a function to find all matched test elements by css selector.
-   * @param css Css root selector of elements. It will locate
+   * Generate a function to find all matched test elements by CSS selector.
+   * @param selector The CSS root selector of elements. It will locate
    * elements under the current root.
    */
-  protected findAll(css: string): () => Promise<TestElement[]>;
+  protected findAll(selector: string): () => Promise<TestElement[]>;
 
   /**
    * Generate a function to find all Component Harnesses under current
    * component harness.
    * @param componentHarness Type of user customized harness.
-   * @param root Css root selector of the new component harnesses. It will
+   * @param root CSS root selector of the new component harnesses. It will
    * locate harnesses under the current root.
    */
   protected findAll<T extends ComponentHarness>(
     componentHarness: ComponentHarnessType<T>,
     root: string): () => Promise<T[]>;
 
   protected findAll<T extends ComponentHarness>(
-    cssOrComponentHarness: string|ComponentHarnessType<T>,
+    selectorOrComponentHarness: string|ComponentHarnessType<T>,
     root?: string): () => Promise<TestElement[]|T[]> {
-    if (typeof cssOrComponentHarness === 'string') {
-      const css = cssOrComponentHarness;
-      return () => this.locator.findAll(css);
+    if (typeof selectorOrComponentHarness === 'string') {
+      const selector = selectorOrComponentHarness;
+      return () => this.locator.querySelectorAll(selector);
     } else {
-      const componentHarness = cssOrComponentHarness;
+      const componentHarness = selectorOrComponentHarness;
       return () => this.locator.loadAll(componentHarness, root as string);
     }
   }

src/cdk-experimental/testing/protractor.ts

@@ -10,7 +10,6 @@
 //  package? It depends on protractor which we don't want to put in the deps for cdk-experimental.
 
 import {browser, by, element as protractorElement, ElementFinder} from 'protractor';
-import {promise as wdpromise} from 'selenium-webdriver';
 
 import {
   ComponentHarness,
@@ -28,33 +27,29 @@ import {
  * loading harness, using the load function that accepts extra searching
  * options.
  * @param componentHarness: Type of user defined harness.
- * @param rootSelector: Optional. Css selector to specify the root of component.
+ * @param rootSelector: Optional. CSS selector to specify the root of component.
  * Set to 'body' by default
  */
 export async function load<T extends ComponentHarness>(
-  componentHarness: ComponentHarnessType<T>,
-  rootSelector: string): Promise<T>;
+    componentHarness: ComponentHarnessType<T>,
+    rootSelector: string): Promise<T>;
 
 /**
  * Component harness factory for protractor.
  * @param componentHarness: Type of user defined harness.
- * @param rootSelector: Optional. Css selector to specify the root of component.
+ * @param rootSelector: Optional. CSS selector to specify the root of component.
  * Set to 'body' by default.
  * @param options Optional. Extra searching options
  */
 export async function load<T extends ComponentHarness>(
-  componentHarness: ComponentHarnessType<T>, rootSelector?: string,
-  options?: Options): Promise<T|null>;
+    componentHarness: ComponentHarnessType<T>, rootSelector?: string,
+    options?: Options): Promise<T|null>;
 
 export async function load<T extends ComponentHarness>(
-  componentHarness: ComponentHarnessType<T>, rootSelector = 'body',
-  options?: Options): Promise<T|null> {
+    componentHarness: ComponentHarnessType<T>, rootSelector = 'body',
+    options?: Options): Promise<T|null> {
   const root = await getElement(rootSelector, undefined, options);
-  if (root === null) {
-    return null;
-  }
-  const locator = new ProtractorLocator(root);
-  return new componentHarness(locator);
+  return root && new componentHarness(new ProtractorLocator(root));
 }
 
 /**
@@ -69,7 +64,7 @@ export function getElementFinder(testElement: TestElement): ElementFinder {
 }
 
 class ProtractorLocator implements Locator {
-  private _root: ProtractorElement;
+  private readonly _root: ProtractorElement;
 
   constructor(private _rootFinder: ElementFinder) {
     this._root = new ProtractorElement(this._rootFinder);
@@ -79,16 +74,13 @@ class ProtractorLocator implements Locator {
     return this._root;
   }
 
-  async find(css: string, options?: Options): Promise<TestElement|null> {
-    const finder = await getElement(css, this._rootFinder, options);
-    if (finder === null) {
-      return null;
-    }
-    return new ProtractorElement(finder);
+  async querySelector(selector: string, options?: Options): Promise<TestElement|null> {
+    const finder = await getElement(selector, this._rootFinder, options);
+    return finder && new ProtractorElement(finder);
   }
 
-  async findAll(css: string): Promise<TestElement[]> {
-    const elementFinders = this._rootFinder.all(by.css(css));
+  async querySelectorAll(selector: string): Promise<TestElement[]> {
+    const elementFinders = this._rootFinder.all(by.css(selector));
     const res: TestElement[] = [];
     await elementFinders.each(el => {
       if (el) {
@@ -99,20 +91,15 @@ class ProtractorLocator implements Locator {
   }
 
   async load<T extends ComponentHarness>(
-    componentHarness: ComponentHarnessType<T>, css: string,
-    options?: Options): Promise<T|null> {
-    const root = await getElement(css, this._rootFinder, options);
-    if (root === null) {
-      return null;
-    }
-    const locator = new ProtractorLocator(root);
-    return new componentHarness(locator);
+      componentHarness: ComponentHarnessType<T>, selector: string,
+      options?: Options): Promise<T|null> {
+    const root = await getElement(selector, this._rootFinder, options);
+    return root && new componentHarness(new ProtractorLocator(root));
   }
 
   async loadAll<T extends ComponentHarness>(
-    componentHarness: ComponentHarnessType<T>,
-    rootSelector: string,
-  ): Promise<T[]> {
+      componentHarness: ComponentHarnessType<T>,
+      rootSelector: string): Promise<T[]> {
     const roots = this._rootFinder.all(by.css(rootSelector));
     const res: T[] = [];
     await roots.each(el => {
@@ -128,77 +115,66 @@ class ProtractorLocator implements Locator {
 class ProtractorElement implements TestElement {
   constructor(readonly element: ElementFinder) {}
 
-  blur(): Promise<void> {
-    return toPromise<void>(this.element['blur']());
+  async blur(): Promise<void> {
+    return this.element['blur']();
   }
 
-  clear(): Promise<void> {
-    return toPromise<void>(this.element.clear());
+  async clear(): Promise<void> {
+    return this.element.clear();
   }
 
-  click(): Promise<void> {
-    return toPromise<void>(this.element.click());
+  async click(): Promise<void> {
+    return this.element.click();
   }
 
-  focus(): Promise<void> {
-    return toPromise<void>(this.element['focus']());
+  async focus(): Promise<void> {
+    return this.element['focus']();
   }
 
-  getCssValue(property: string): Promise<string> {
-    return toPromise<string>(this.element.getCssValue(property));
+  async getCssValue(property: string): Promise<string> {
+    return this.element.getCssValue(property);
   }
 
   async hover(): Promise<void> {
-    return toPromise<void>(browser.actions()
-      .mouseMove(await this.element.getWebElement())
-      .perform());
+    return browser.actions()
+        .mouseMove(await this.element.getWebElement())
+        .perform();
   }
 
-  sendKeys(keys: string): Promise<void> {
-    return toPromise<void>(this.element.sendKeys(keys));
+  async sendKeys(keys: string): Promise<void> {
+    return this.element.sendKeys(keys);
   }
 
-  text(): Promise<string> {
-    return toPromise(this.element.getText());
+  async text(): Promise<string> {
+    return this.element.getText();
   }
 
-  getAttribute(name: string): Promise<string|null> {
-    return toPromise(this.element.getAttribute(name));
+  async getAttribute(name: string): Promise<string|null> {
+    return this.element.getAttribute(name);
   }
 }
 
-function toPromise<T>(p: wdpromise.Promise<T>): Promise<T> {
-  return new Promise<T>((resolve, reject) => {
-    p.then(resolve, reject);
-  });
-}
-
 /**
- * Get an element finder based on the css selector and root element.
+ * Get an element finder based on the CSS selector and root element.
  * Note that it will check whether the element is present only when
  * Options.allowNull is set. This is for performance purpose.
- * @param css the css selector
+ * @param selector The CSS selector
  * @param root Optional Search element under the root element. If not set,
  * search element globally. If options.global is set, root is ignored.
  * @param options Optional, extra searching options
  */
-async function getElement(css: string, root?: ElementFinder, options?: Options):
+async function getElement(selector: string, root?: ElementFinder, options?: Options):
   Promise<ElementFinder|null> {
   const useGlobalRoot = options && !!options.global;
-  const elem = root === undefined || useGlobalRoot ? protractorElement(by.css(css)) :
-    root.element(by.css(css));
+  const elem = root === undefined || useGlobalRoot ?
+      protractorElement(by.css(selector)) : root.element(by.css(selector));
   const allowNull = options !== undefined && options.allowNull !== undefined ?
-    options.allowNull :
-    undefined;
-  if (allowNull !== undefined) {
-    // Only check isPresent when allowNull is set
-    if (!(await elem.isPresent())) {
-      if (allowNull) {
-        return null;
-      }
-      throw new Error('Cannot find element based on the css selector: ' + css);
+      options.allowNull : undefined;
+  if (allowNull !== undefined && !(await elem.isPresent())) {
+    if (allowNull) {
+      return null;
     }
-    return elem;
+    throw new Error('Cannot find element based on the CSS selector: ' + selector);
   }
   return elem;
 }