From f677cb9ace9b2b263516ee15c3109a8af925b4be Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 29 Jul 2020 22:53:42 +0200 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 | 9 +++++++- src/cdk/testing/public-api.ts | 1 + .../selenium-web-driver-element.ts | 9 +++++++- src/cdk/testing/test-element-errors.ts | 15 ++++++++++++ src/cdk/testing/test-element.ts | 6 +++-- .../testbed/fake-events/type-in-element.ts | 23 ++++++++++++++----- .../testing/tests/cross-environment.spec.ts | 20 ++++++++++++++++ tools/public_api_guard/cdk/testing.md | 3 +++ 8 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 src/cdk/testing/test-element-errors.ts diff --git a/src/cdk/testing/protractor/protractor-element.ts b/src/cdk/testing/protractor/protractor-element.ts index 48fdb803f77a..32bc5eab651b 100644 --- a/src/cdk/testing/protractor/protractor-element.ts +++ b/src/cdk/testing/protractor/protractor-element.ts @@ -9,6 +9,7 @@ import { _getTextWithExcludedElements, ElementDimensions, + getNoKeysSpecifiedError, ModifierKeys, TestElement, TestKey, @@ -161,7 +162,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 { @@ -177,6 +178,12 @@ export class ProtractorElement implements TestElement { // so avoid it if no modifier keys are required. .map(k => (modifierKeys.length > 0 ? Key.chord(...modifierKeys, k) : 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/public-api.ts b/src/cdk/testing/public-api.ts index 9a535613114b..4e162e3f47a1 100644 --- a/src/cdk/testing/public-api.ts +++ b/src/cdk/testing/public-api.ts @@ -9,6 +9,7 @@ export * from './component-harness'; export * from './harness-environment'; export * from './test-element'; +export * from './test-element-errors'; export * from './element-dimensions'; export * from './text-filtering'; export * from './change-detection'; diff --git a/src/cdk/testing/selenium-webdriver/selenium-web-driver-element.ts b/src/cdk/testing/selenium-webdriver/selenium-web-driver-element.ts index b14c710da74e..22165c28dc24 100644 --- a/src/cdk/testing/selenium-webdriver/selenium-web-driver-element.ts +++ b/src/cdk/testing/selenium-webdriver/selenium-web-driver-element.ts @@ -10,6 +10,7 @@ import { _getTextWithExcludedElements, ElementDimensions, EventData, + getNoKeysSpecifiedError, ModifierKeys, TestElement, TestKey, @@ -111,7 +112,7 @@ export class SeleniumWebDriverElement 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 { @@ -127,6 +128,12 @@ export class SeleniumWebDriverElement implements TestElement { // so avoid it if no modifier keys are required. .map(k => (modifierKeys.length > 0 ? webdriver.Key.chord(...modifierKeys, k) : 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(); + } + await this.element().sendKeys(...keys); await this._stabilize(); } diff --git a/src/cdk/testing/test-element-errors.ts b/src/cdk/testing/test-element-errors.ts new file mode 100644 index 000000000000..2ec44f4a68b7 --- /dev/null +++ b/src/cdk/testing/test-element-errors.ts @@ -0,0 +1,15 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +/** + * 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/test-element.ts b/src/cdk/testing/test-element.ts index 844a94925756..368ddc9742d4 100644 --- a/src/cdk/testing/test-element.ts +++ b/src/cdk/testing/test-element.ts @@ -119,12 +119,14 @@ 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. Note that some environments cannot * reproduce native browser behavior for keyboard shortcuts such as Tab, Ctrl + A, etc. + * @throws An error 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. + * 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. + * @throws An error if no keys have been specified. */ sendKeys(modifiers: ModifierKeys, ...keys: (string | TestKey)[]): Promise; 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 5cbb4879612c..69a43acf5791 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 {PERIOD} from '@angular/cdk/keycodes'; import {dispatchFakeEvent, dispatchKeyboardEvent} from './dispatch-events'; import {triggerFocus} from './element-focus'; @@ -32,7 +32,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. @@ -44,7 +44,7 @@ export function typeInElement( ): 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. @@ -57,11 +57,16 @@ export function typeInElement( ...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 { @@ -78,13 +83,19 @@ export function typeInElement(element: HTMLElement, ...modifiersAndKeys: any) { ) .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(); + } + // We simulate the user typing in a value by incrementally assigning the value below. The problem // is that for some input types, the browser won't allow for an invalid value to be set via the // `value` property which will always be the case when going character-by-character. If we detect // such an input, we have to set the value all at once or listeners to the `input` event (e.g. // the `ReactiveFormsModule` uses such an approach) won't receive the correct value. const enterValueIncrementally = - inputType === 'number' && keys.length > 0 + inputType === 'number' ? // The value can be set character by character in number inputs if it doesn't have any decimals. keys.every(key => key.key !== '.' && key.keyCode !== PERIOD) : incrementalInputTypes.has(inputType); diff --git a/src/cdk/testing/tests/cross-environment.spec.ts b/src/cdk/testing/tests/cross-environment.spec.ts index e6d6827670ce..dd026a5f6dce 100644 --- a/src/cdk/testing/tests/cross-environment.spec.ts +++ b/src/cdk/testing/tests/cross-environment.spec.ts @@ -9,6 +9,7 @@ import { ComponentHarness, ComponentHarnessConstructor, + getNoKeysSpecifiedError, HarnessLoader, HarnessPredicate, parallel, @@ -345,6 +346,18 @@ export function crossEnvironmentSpecs( harness = await getMainComponentHarnessFromEnvironment(); }); + async function expectAsyncError(fn: () => Promise, expected: Error) { + let error: unknown | null = null; + try { + await fn(); + } catch (e: unknown) { + error = e; + } + expect(error).not.toBe(null); + expect(error instanceof Error).toBe(true); + expect((error as Error).message).toBe(expected.message); + } + it('should be able to clear', async () => { const input = await harness.input(); await input.sendKeys('Yi'); @@ -354,6 +367,13 @@ export function crossEnvironmentSpecs( 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()); + await expectAsyncError(() => input.sendKeys(''), getNoKeysSpecifiedError()); + await expectAsyncError(() => input.sendKeys('', ''), getNoKeysSpecifiedError()); + }); + it('should be able to click', async () => { const counter = await harness.counter(); expect(await counter.text()).toBe('0'); diff --git a/tools/public_api_guard/cdk/testing.md b/tools/public_api_guard/cdk/testing.md index 3d7a0ad36201..6abc763acfae 100644 --- a/tools/public_api_guard/cdk/testing.md +++ b/tools/public_api_guard/cdk/testing.md @@ -76,6 +76,9 @@ export type EventData = string | number | boolean | undefined | null | EventData [key: string]: EventData; }; +// @public +export function getNoKeysSpecifiedError(): Error; + // @public export function _getTextWithExcludedElements(element: Element, excludeSelector: string): string;