From 827c4852fe8ca75fa36e45a351944e6a5aed1dfe Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 28 Jan 2020 15:20:41 +0100 Subject: [PATCH] fix(cdk/testing): TestElement sendKeys method should throw if no keys have been specified Previously, calling `sendKeys` without any keys resulted in a runtime exception `Cannot read X of undefined` error being thrown. This is because the first element of the passed arguments to `sendKeys` has been considered always truthy. This assumption is wrong since arbitrary amount of arguments can be passed due to the spread parameter. To ensure consistent and reasonable behavior of this function, we fix the runtime exception mentioned above, but throw if no keys have been determined (not necessarily only if the arguments length is zero). --- .../testing/protractor/protractor-element.ts | 16 ++++++++++++++-- src/cdk/testing/test-element.ts | 12 ++++++++++-- .../testbed/fake-events/type-in-element.ts | 17 ++++++++++++----- src/cdk/testing/tests/protractor.e2e.spec.ts | 8 ++++++++ src/cdk/testing/tests/testbed.spec.ts | 9 ++++++++- 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/cdk/testing/protractor/protractor-element.ts b/src/cdk/testing/protractor/protractor-element.ts index 7167f45363f0..501b75431ce8 100644 --- a/src/cdk/testing/protractor/protractor-element.ts +++ b/src/cdk/testing/protractor/protractor-element.ts @@ -6,7 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ -import {ElementDimensions, ModifierKeys, TestElement, TestKey} from '@angular/cdk/testing'; +import { + ElementDimensions, + getNoKeysSpecifiedError, + ModifierKeys, + TestElement, + TestKey +} from '@angular/cdk/testing'; import {browser, ElementFinder, Key} from 'protractor'; /** Maps the `TestKey` constants to Protractor's `Key` constants. */ @@ -100,7 +106,7 @@ export class ProtractorElement implements TestElement { const first = modifiersAndKeys[0]; let modifiers: ModifierKeys; let rest: (string | TestKey)[]; - if (typeof first !== 'string' && typeof first !== 'number') { + if (first !== undefined && typeof first !== 'string' && typeof first !== 'number') { modifiers = first; rest = modifiersAndKeys.slice(1); } else { @@ -113,6 +119,12 @@ export class ProtractorElement implements TestElement { .reduce((arr, k) => arr.concat(k), []) .map(k => Key.chord(...modifierKeys, k)); + // Throw an error if no keys have been specified. Calling this function with no + // keys should not result in a focus event being dispatched unexpectedly. + if (keys.length === 0) { + throw getNoKeysSpecifiedError(); + } + return this.element.sendKeys(...keys); } diff --git a/src/cdk/testing/test-element.ts b/src/cdk/testing/test-element.ts index 6c0329ea6e4b..762c650ac661 100644 --- a/src/cdk/testing/test-element.ts +++ b/src/cdk/testing/test-element.ts @@ -84,13 +84,13 @@ export interface TestElement { /** * 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. + * and attempts to add the string to the Element's value. Throws if no keys have been specified. */ sendKeys(...keys: (string | TestKey)[]): Promise; /** * 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. + * and attempts to add the string to the Element's value. Throws if no keys have been specified. */ sendKeys(modifiers: ModifierKeys, ...keys: (string | TestKey)[]): Promise; @@ -115,3 +115,11 @@ export interface TestElement { /** Checks whether the element is focused. */ isFocused(): Promise; } + +/** + * Returns an error which reports that no keys have been specified. + * @docs-private + */ +export function getNoKeysSpecifiedError() { + return Error('No keys have been specified.'); +} diff --git a/src/cdk/testing/testbed/fake-events/type-in-element.ts b/src/cdk/testing/testbed/fake-events/type-in-element.ts index 41e715a95ac6..703bd6670276 100644 --- a/src/cdk/testing/testbed/fake-events/type-in-element.ts +++ b/src/cdk/testing/testbed/fake-events/type-in-element.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ModifierKeys} from '@angular/cdk/testing'; +import {getNoKeysSpecifiedError, ModifierKeys} from '@angular/cdk/testing'; import {dispatchFakeEvent, dispatchKeyboardEvent} from './dispatch-events'; import {triggerFocus} from './element-focus'; @@ -20,7 +20,7 @@ export function isTextInput(element: Element): element is HTMLInputElement | HTM } /** - * Focuses an input, sets its value and dispatches + * If keys have been specified, focuses an input, sets its value and dispatches * the `input` event, simulating the user typing. * @param element Element onto which to set the value. * @param keys The keys to send to the element. @@ -30,7 +30,7 @@ export function typeInElement( element: HTMLElement, ...keys: (string | {keyCode?: number, key?: string})[]): void; /** - * Focuses an input, sets its value and dispatches + * If keys have been specified, focuses an input, sets its value and dispatches * the `input` event, simulating the user typing. * @param element Element onto which to set the value. * @param modifiers Modifier keys that are held while typing. @@ -40,11 +40,12 @@ export function typeInElement( export function typeInElement(element: HTMLElement, modifiers: ModifierKeys, ...keys: (string | {keyCode?: number, key?: string})[]): void; -export function typeInElement(element: HTMLElement, ...modifiersAndKeys: any) { +export function typeInElement(element: HTMLElement, ...modifiersAndKeys: any[]) { const first = modifiersAndKeys[0]; let modifiers: ModifierKeys; let rest: (string | {keyCode?: number, key?: string})[]; - if (typeof first !== 'string' && first.keyCode === undefined && first.key === undefined) { + if (first !== undefined && typeof first !== 'string' && first.keyCode === undefined && + first.key === undefined) { modifiers = first; rest = modifiersAndKeys.slice(1); } else { @@ -56,6 +57,12 @@ export function typeInElement(element: HTMLElement, ...modifiersAndKeys: any) { k.split('').map(c => ({keyCode: c.toUpperCase().charCodeAt(0), key: c})) : [k]) .reduce((arr, k) => arr.concat(k), []); + // Throw an error if no keys have been specified. Calling this function with no + // keys should not result in a focus event being dispatched unexpectedly. + if (keys.length === 0) { + throw getNoKeysSpecifiedError(); + } + triggerFocus(element); for (const key of keys) { dispatchKeyboardEvent(element, 'keydown', key.keyCode, key.key, element, modifiers); diff --git a/src/cdk/testing/tests/protractor.e2e.spec.ts b/src/cdk/testing/tests/protractor.e2e.spec.ts index af5ca877f160..720a79f3ac91 100644 --- a/src/cdk/testing/tests/protractor.e2e.spec.ts +++ b/src/cdk/testing/tests/protractor.e2e.spec.ts @@ -1,6 +1,7 @@ import { ComponentHarness, ComponentHarnessConstructor, + getNoKeysSpecifiedError, HarnessLoader, TestElement } from '@angular/cdk/testing'; @@ -197,6 +198,13 @@ describe('ProtractorHarnessEnvironment', () => { expect(await input.getProperty('value')).toBe(''); }); + it('sendKeys method should throw if no keys have been specified', async () => { + const input = await harness.input(); + await expectAsyncError(() => input.sendKeys(), getNoKeysSpecifiedError().toString()); + await expectAsyncError(() => input.sendKeys(''), getNoKeysSpecifiedError().toString()); + await expectAsyncError(() => input.sendKeys('', ''), getNoKeysSpecifiedError().toString()); + }); + it('should be able to click', async () => { const counter = await harness.counter(); expect(await counter.text()).toBe('0'); diff --git a/src/cdk/testing/tests/testbed.spec.ts b/src/cdk/testing/tests/testbed.spec.ts index ee9cc26e6104..e8519025aee8 100644 --- a/src/cdk/testing/tests/testbed.spec.ts +++ b/src/cdk/testing/tests/testbed.spec.ts @@ -1,6 +1,7 @@ import { ComponentHarness, ComponentHarnessConstructor, + getNoKeysSpecifiedError, HarnessLoader, TestElement } from '@angular/cdk/testing'; @@ -16,7 +17,6 @@ import {TestMainComponent} from './test-main-component'; function activeElementText() { return document.activeElement && (document.activeElement as HTMLElement).innerText || ''; } - describe('TestbedHarnessEnvironment', () => { let fixture: ComponentFixture<{}>; @@ -294,6 +294,13 @@ describe('TestbedHarnessEnvironment', () => { expect(await input.getProperty('value')).toBe(''); }); + it('sendKeys method should throw if no keys have been specified', async () => { + const input = await harness.input(); + await expectAsyncError(() => input.sendKeys(), getNoKeysSpecifiedError().toString()); + await expectAsyncError(() => input.sendKeys(''), getNoKeysSpecifiedError().toString()); + await expectAsyncError(() => input.sendKeys('', ''), getNoKeysSpecifiedError().toString()); + }); + it('should be able to click', async () => { const counter = await harness.counter(); expect(await counter.text()).toBe('0');