From f98526a2c90ee9dd97993a3324b262c29f5d1657 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 27 Sep 2021 12:15:17 -0700 Subject: [PATCH] Added unit tests to protect against broken startup sequence Summary: Add unit tests to test errors thrown from plugins during initialisation, as follow up for D31127969 (https://github.com/facebook/flipper/commit/a72e46c79291301da6a6879bc77bad1eb28a54f7). From the 4 tests cases added (first load plugin then device, device then plugin, first load plugin then client, first client then plugin), the first case was indeed failing before the mentioned diff and not registering the device. Reviewed By: passy Differential Revision: D31142583 fbshipit-source-id: 8e44c667316192231d1bb5e4d76c5bf1207ba835 --- .../reducers/__tests__/connections.node.tsx | 99 ++++++++++++++++++- .../flipper-plugin/src/__tests__/api.node.tsx | 1 + desktop/flipper-plugin/src/index.ts | 1 + .../src/test-utils/test-utils.tsx | 40 ++++++++ 4 files changed, 138 insertions(+), 3 deletions(-) diff --git a/desktop/app/src/reducers/__tests__/connections.node.tsx b/desktop/app/src/reducers/__tests__/connections.node.tsx index b415da72d..bd5a8fe6d 100644 --- a/desktop/app/src/reducers/__tests__/connections.node.tsx +++ b/desktop/app/src/reducers/__tests__/connections.node.tsx @@ -9,15 +9,23 @@ import reducer from '../connections'; import {State, selectPlugin} from '../connections'; -import {_setFlipperLibImplementation} from 'flipper-plugin'; -import {createMockFlipperLib} from 'flipper-plugin/src/test-utils/test-utils'; +import { + _SandyPluginDefinition, + _setFlipperLibImplementation, + TestUtils, + MockedConsole, +} from 'flipper-plugin'; import {TestDevice} from '../../test-utils/TestDevice'; +import {createMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWithPlugin'; +let mockedConsole: MockedConsole; beforeEach(() => { - _setFlipperLibImplementation(createMockFlipperLib()); + mockedConsole = TestUtils.mockConsole(); + _setFlipperLibImplementation(TestUtils.createMockFlipperLib()); }); afterEach(() => { + mockedConsole.unmock(); _setFlipperLibImplementation(undefined); }); @@ -79,3 +87,88 @@ test('selectPlugin sets deepLinkPayload correctly', () => { ); expect(state.deepLinkPayload).toBe('myPayload'); }); + +test('can handle plugins that throw at start', async () => { + const TestPlugin = new _SandyPluginDefinition( + TestUtils.createMockPluginDetails(), + { + Component() { + return null; + }, + plugin() { + throw new Error('Broken plugin'); + }, + }, + ); + + const {client, store, createClient, createDevice} = + await createMockFlipperWithPlugin(TestPlugin); + + // not initialized + expect(client.sandyPluginStates.get(TestPlugin.id)).toBe(undefined); + + expect(store.getState().connections.clients.length).toBe(1); + expect(client.connected.get()).toBe(true); + + expect((console.error as any).mock.calls[0]).toMatchInlineSnapshot(` + Array [ + "Failed to start plugin 'TestPlugin': ", + [Error: Broken plugin], + ] + `); + + const device2 = await createDevice({}); + const client2 = await createClient(device2, client.query.app); + + expect((console.error as any).mock.calls[1]).toMatchInlineSnapshot(` + Array [ + "Failed to start plugin 'TestPlugin': ", + [Error: Broken plugin], + ] + `); + expect(store.getState().connections.clients.length).toBe(2); + expect(client2.connected.get()).toBe(true); + expect(client2.sandyPluginStates.size).toBe(0); +}); + +test('can handle device plugins that throw at start', async () => { + const TestPlugin = new _SandyPluginDefinition( + TestUtils.createMockPluginDetails(), + { + Component() { + return null; + }, + devicePlugin() { + throw new Error('Broken device plugin'); + }, + }, + ); + + const {device, store, createDevice} = await createMockFlipperWithPlugin( + TestPlugin, + ); + + expect(mockedConsole.errorCalls[0]).toMatchInlineSnapshot(` + Array [ + "Failed to start device plugin 'TestPlugin': ", + [Error: Broken device plugin], + ] + `); + + // not initialized + expect(device.sandyPluginStates.get(TestPlugin.id)).toBe(undefined); + + expect(store.getState().connections.devices.length).toBe(1); + expect(device.connected.get()).toBe(true); + + const device2 = await createDevice({}); + expect(store.getState().connections.devices.length).toBe(2); + expect(device2.connected.get()).toBe(true); + expect(mockedConsole.errorCalls[1]).toMatchInlineSnapshot(` + Array [ + "Failed to start device plugin 'TestPlugin': ", + [Error: Broken device plugin], + ] + `); + expect(device2.sandyPluginStates.size).toBe(0); +}); diff --git a/desktop/flipper-plugin/src/__tests__/api.node.tsx b/desktop/flipper-plugin/src/__tests__/api.node.tsx index e1331fd08..5314cd269 100644 --- a/desktop/flipper-plugin/src/__tests__/api.node.tsx +++ b/desktop/flipper-plugin/src/__tests__/api.node.tsx @@ -102,6 +102,7 @@ test('Correct top level API exposed', () => { "LogTypes", "Logger", "MenuEntry", + "MockedConsole", "NormalizedMenuEntry", "Notification", "PluginClient", diff --git a/desktop/flipper-plugin/src/index.ts b/desktop/flipper-plugin/src/index.ts index 96126bb51..b970e0d01 100644 --- a/desktop/flipper-plugin/src/index.ts +++ b/desktop/flipper-plugin/src/index.ts @@ -13,6 +13,7 @@ export const styled = styledImport; import './plugin/PluginBase'; import * as TestUtilites from './test-utils/test-utils'; +export {MockedConsole} from './test-utils/test-utils'; export { SandyPluginInstance as _SandyPluginInstance, diff --git a/desktop/flipper-plugin/src/test-utils/test-utils.tsx b/desktop/flipper-plugin/src/test-utils/test-utils.tsx index 82c506101..5e1162640 100644 --- a/desktop/flipper-plugin/src/test-utils/test-utils.tsx +++ b/desktop/flipper-plugin/src/test-utils/test-utils.tsx @@ -40,6 +40,7 @@ import {FlipperLib} from '../plugin/FlipperLib'; import {stubLogger} from '../utils/Logger'; import {Idler} from '../utils/Idler'; import {createState} from '../state/atom'; +import baseMockConsole from 'jest-mock-console'; type Renderer = RenderResult; @@ -532,3 +533,42 @@ function createStubIdler(): Idler { }, }; } + +/** + * Mockes the current console. Inspect results through e.g. + * console.errorCalls etc. + * + * Or, alternatively, expect(mockedConsole.error).toBeCalledWith... + * + * Don't forgot to call .unmock when done! + */ +export function mockConsole() { + const restoreConsole = baseMockConsole(); + // The mocked console methods, make sure they remain available after unmocking + const {log, error, warn} = console as any; + return { + get logCalls(): any[][] { + return log.mock.calls; + }, + get errorCalls(): any[][] { + return error.mock.calls; + }, + get warnCalls(): any[][] { + return warn.mock.calls; + }, + get log(): jest.Mock { + return log as any; + }, + get warn(): jest.Mock { + return warn as any; + }, + get error(): jest.Mock { + return error as any; + }, + unmock() { + restoreConsole(); + }, + }; +} + +export type MockedConsole = ReturnType;