From d538b66088d710ffbc4887168e0fdff2a8ce1a59 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Tue, 4 Aug 2020 07:44:56 -0700 Subject: [PATCH] Cleaned up deviceType enum Summary: The device type enum was mixing two different concepts (emulator vs physical) and (archived vs not), and we already have a separate `isArchived` field. So cleaned this up to not leak it into sandy. If anybody can think of any unforeseen consequences of this, lemme know :) Reviewed By: jknoxville Differential Revision: D22763506 fbshipit-source-id: bd2f7dbd1d2d2e6942ba7c6ddd8dc91ee34d591d --- desktop/app/src/devices/AndroidDevice.tsx | 4 ++-- desktop/app/src/devices/ArchivedDevice.tsx | 21 +++--------------- desktop/app/src/devices/BaseDevice.tsx | 7 +----- .../devices/FlipperSelfInspectionDevice.tsx | 3 ++- desktop/app/src/devices/IOSDevice.tsx | 3 +-- desktop/app/src/dispatcher/iOSDevice.tsx | 2 +- .../src/utils/__tests__/exportData.node.tsx | 22 +++++++++---------- desktop/flipper-plugin/src/index.ts | 1 + .../src/plugin/DevicePlugin.tsx | 16 ++++++++++---- .../src/test-utils/test-utils.tsx | 2 ++ 10 files changed, 36 insertions(+), 45 deletions(-) diff --git a/desktop/app/src/devices/AndroidDevice.tsx b/desktop/app/src/devices/AndroidDevice.tsx index 87d796816..46f7ac7bd 100644 --- a/desktop/app/src/devices/AndroidDevice.tsx +++ b/desktop/app/src/devices/AndroidDevice.tsx @@ -7,12 +7,12 @@ * @format */ -import BaseDevice, {DeviceType} from './BaseDevice'; +import BaseDevice from './BaseDevice'; import adb, {Client as ADBClient} from 'adbkit'; import {Priority} from 'adbkit-logcat'; import ArchivedDevice from './ArchivedDevice'; import {createWriteStream} from 'fs'; -import {LogLevel} from 'flipper-plugin'; +import {LogLevel, DeviceType} from 'flipper-plugin'; const DEVICE_RECORDING_DIR = '/sdcard/flipper_recorder'; diff --git a/desktop/app/src/devices/ArchivedDevice.tsx b/desktop/app/src/devices/ArchivedDevice.tsx index ce7d743f1..d8eb8495b 100644 --- a/desktop/app/src/devices/ArchivedDevice.tsx +++ b/desktop/app/src/devices/ArchivedDevice.tsx @@ -7,21 +7,11 @@ * @format */ -import {DeviceLogEntry} from 'flipper-plugin'; +import {DeviceLogEntry, DeviceType} from 'flipper-plugin'; import BaseDevice from './BaseDevice'; -import {DeviceType, OS, DeviceShell} from './BaseDevice'; +import {OS, DeviceShell} from './BaseDevice'; import {SupportFormRequestDetailsState} from '../reducers/supportForm'; -function normalizeArchivedDeviceType(deviceType: DeviceType): DeviceType { - let archivedDeviceType = deviceType; - if (archivedDeviceType === 'emulator') { - archivedDeviceType = 'archivedEmulator'; - } else if (archivedDeviceType === 'physical') { - archivedDeviceType = 'archivedPhysical'; - } - return archivedDeviceType; -} - export default class ArchivedDevice extends BaseDevice { constructor(options: { serial: string; @@ -33,12 +23,7 @@ export default class ArchivedDevice extends BaseDevice { source?: string; supportRequestDetails?: SupportFormRequestDetailsState; }) { - super( - options.serial, - normalizeArchivedDeviceType(options.deviceType), - options.title, - options.os, - ); + super(options.serial, options.deviceType, options.title, options.os); this.logs = options.logEntries; this.source = options.source || ''; this.supportRequestDetails = options.supportRequestDetails; diff --git a/desktop/app/src/devices/BaseDevice.tsx b/desktop/app/src/devices/BaseDevice.tsx index ad40e3ec1..cb06092d2 100644 --- a/desktop/app/src/devices/BaseDevice.tsx +++ b/desktop/app/src/devices/BaseDevice.tsx @@ -14,6 +14,7 @@ import { DeviceLogEntry, SandyDevicePluginInstance, SandyPluginDefinition, + DeviceType, } from 'flipper-plugin'; import {DevicePluginMap, FlipperDevicePlugin} from '../plugin'; @@ -23,12 +24,6 @@ export type DeviceShell = { stdin: stream.Writable; }; -export type DeviceType = - | 'emulator' - | 'physical' - | 'archivedEmulator' - | 'archivedPhysical'; - export type DeviceExport = { os: OS; title: string; diff --git a/desktop/app/src/devices/FlipperSelfInspectionDevice.tsx b/desktop/app/src/devices/FlipperSelfInspectionDevice.tsx index be93bc4d0..61761c665 100644 --- a/desktop/app/src/devices/FlipperSelfInspectionDevice.tsx +++ b/desktop/app/src/devices/FlipperSelfInspectionDevice.tsx @@ -7,7 +7,8 @@ * @format */ -import BaseDevice, {OS, DeviceType} from './BaseDevice'; +import BaseDevice, {OS} from './BaseDevice'; +import {DeviceType} from 'flipper-plugin'; export default class FlipperSelfInspectionDevice extends BaseDevice { constructor(serial: string, deviceType: DeviceType, title: string, os: OS) { diff --git a/desktop/app/src/devices/IOSDevice.tsx b/desktop/app/src/devices/IOSDevice.tsx index e2ba3b459..1515cccfc 100644 --- a/desktop/app/src/devices/IOSDevice.tsx +++ b/desktop/app/src/devices/IOSDevice.tsx @@ -7,8 +7,7 @@ * @format */ -import {LogLevel, DeviceLogEntry} from 'flipper-plugin'; -import {DeviceType} from './BaseDevice'; +import {LogLevel, DeviceLogEntry, DeviceType} from 'flipper-plugin'; import child_process, {ChildProcess} from 'child_process'; import BaseDevice from './BaseDevice'; import JSONStream from 'JSONStream'; diff --git a/desktop/app/src/dispatcher/iOSDevice.tsx b/desktop/app/src/dispatcher/iOSDevice.tsx index 9ff132bfa..f2b173a1d 100644 --- a/desktop/app/src/dispatcher/iOSDevice.tsx +++ b/desktop/app/src/dispatcher/iOSDevice.tsx @@ -11,7 +11,7 @@ import {ChildProcess} from 'child_process'; import {Store} from '../reducers/index'; import {setXcodeDetected} from '../reducers/application'; import {Logger} from '../fb-interfaces/Logger'; -import {DeviceType} from '../devices/BaseDevice'; +import {DeviceType} from 'flipper-plugin'; import {promisify} from 'util'; import path from 'path'; import child_process from 'child_process'; diff --git a/desktop/app/src/utils/__tests__/exportData.node.tsx b/desktop/app/src/utils/__tests__/exportData.node.tsx index 86362ac6e..8949bef9d 100644 --- a/desktop/app/src/utils/__tests__/exportData.node.tsx +++ b/desktop/app/src/utils/__tests__/exportData.node.tsx @@ -104,8 +104,8 @@ test('test generateClientIndentifierWithSalt helper function', () => { }); const identifier = generateClientIdentifier(device, 'app'); const saltIdentifier = generateClientIdentifierWithSalt(identifier, 'salt'); - expect(saltIdentifier).toEqual('app#iOS#archivedEmulator#salt-serial'); - expect(identifier).toEqual('app#iOS#archivedEmulator#serial'); + expect(saltIdentifier).toEqual('app#iOS#emulator#salt-serial'); + expect(identifier).toEqual('app#iOS#emulator#serial'); }); test('test generateClientFromClientWithSalt helper function', () => { @@ -120,20 +120,20 @@ test('test generateClientFromClientWithSalt helper function', () => { const client = generateClientFromDevice(device, 'app'); const saltedClient = generateClientFromClientWithSalt(client, 'salt'); expect(saltedClient).toEqual({ - id: 'app#iOS#archivedEmulator#salt-serial', + id: 'app#iOS#emulator#salt-serial', query: { app: 'app', os: 'iOS', - device: 'archivedEmulator', + device: 'emulator', device_id: 'salt-serial', }, }); expect(client).toEqual({ - id: 'app#iOS#archivedEmulator#serial', + id: 'app#iOS#emulator#serial', query: { app: 'app', os: 'iOS', - device: 'archivedEmulator', + device: 'emulator', device_id: 'serial', }, }); @@ -150,11 +150,11 @@ test('test generateClientFromDevice helper function', () => { }); const client = generateClientFromDevice(device, 'app'); expect(client).toEqual({ - id: 'app#iOS#archivedEmulator#serial', + id: 'app#iOS#emulator#serial', query: { app: 'app', os: 'iOS', - device: 'archivedEmulator', + device: 'emulator', device_id: 'serial', }, }); @@ -170,7 +170,7 @@ test('test generateClientIdentifier helper function', () => { screenshotHandle: null, }); const identifier = generateClientIdentifier(device, 'app'); - expect(identifier).toEqual('app#iOS#archivedEmulator#serial'); + expect(identifier).toEqual('app#iOS#emulator#serial'); }); test('test generateNotifications helper function', () => { @@ -231,7 +231,7 @@ test('test processStore function for an iOS device connected', async () => { } const {serial, deviceType, title, os} = device; expect(serial).toEqual('salt-serial'); - expect(deviceType).toEqual('archivedEmulator'); + expect(deviceType).toEqual('emulator'); expect(title).toEqual('TestiPhone'); expect(os).toEqual('iOS'); const {pluginStates, activeNotifications} = json.store; @@ -1045,7 +1045,7 @@ test('Sandy plugins are imported properly', async () => { }, ], device: { - deviceType: 'archivedPhysical', + deviceType: 'physical', logs: [], os: 'Android', serial: '2e52cea6-94b0-4ea1-b9a8-c9135ede14ca-serial', diff --git a/desktop/flipper-plugin/src/index.ts b/desktop/flipper-plugin/src/index.ts index 6b0486f17..bd85c070c 100644 --- a/desktop/flipper-plugin/src/index.ts +++ b/desktop/flipper-plugin/src/index.ts @@ -20,6 +20,7 @@ export { DevicePluginClient, LogLevel, SandyDevicePluginInstance, + DeviceType, } from './plugin/DevicePlugin'; export {SandyPluginDefinition} from './plugin/SandyPluginDefinition'; export {SandyPluginRenderer} from './plugin/PluginRenderer'; diff --git a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx index aabf516e6..0f9ed440e 100644 --- a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx +++ b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx @@ -32,10 +32,14 @@ export type LogLevel = | 'fatal'; export interface Device { - isArchived: boolean; + readonly isArchived: boolean; + readonly os: string; + readonly deviceType: DeviceType; onLogEntry(cb: DeviceLogListener): () => void; } +export type DeviceType = 'emulator' | 'physical'; + export type DevicePluginPredicate = (device: Device) => boolean; export type DevicePluginFactory = (client: DevicePluginClient) => object; @@ -48,7 +52,9 @@ export interface DevicePluginClient extends BasePluginClient { * Wrapper interface around BaseDevice in Flipper */ export interface RealFlipperDevice { + os: string; isArchived: boolean; + deviceType: DeviceType; addLogListener(callback: DeviceLogListener): Symbol; removeLogListener(id: Symbol): void; addLogEntry(entry: DeviceLogEntry): void; @@ -69,9 +75,11 @@ export class SandyDevicePluginInstance extends BasePluginInstance { ) { super(definition, initialStates); const device: Device = { - get isArchived() { - return realDevice.isArchived; - }, + // N.B. we model OS as string, not as enum, to make custom device types possible in the future + os: realDevice.os, + isArchived: realDevice.isArchived, + deviceType: realDevice.deviceType, + onLogEntry(cb) { const handle = realDevice.addLogListener(cb); return () => { diff --git a/desktop/flipper-plugin/src/test-utils/test-utils.tsx b/desktop/flipper-plugin/src/test-utils/test-utils.tsx index e78c8e8b8..7c975f650 100644 --- a/desktop/flipper-plugin/src/test-utils/test-utils.tsx +++ b/desktop/flipper-plugin/src/test-utils/test-utils.tsx @@ -341,6 +341,8 @@ export function createMockPluginDetails( function createMockDevice(options?: StartPluginOptions): RealFlipperDevice { const logListeners: (undefined | DeviceLogListener)[] = []; return { + os: 'Android', + deviceType: 'emulator', isArchived: !!options?.isArchived, addLogListener(cb) { logListeners.push(cb);