From c11618949ebdf663f0ea1beb7f734e677c2bca0a Mon Sep 17 00:00:00 2001 From: Sanniti Date: Wed, 27 May 2020 18:53:48 -0400 Subject: [PATCH 01/10] added cache disable logic. added toggle --- app-shell/src/discovery.js | 9 +++- .../components/NetworkSettingsCard/index.js | 2 + .../toggleDiscoveryCaching.js | 48 +++++++++++++++++++ app/src/config/actions.js | 6 +++ 4 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 app/src/components/NetworkSettingsCard/toggleDiscoveryCaching.js diff --git a/app-shell/src/discovery.js b/app-shell/src/discovery.js index eae1f341d0a..9a6fdeb7204 100644 --- a/app-shell/src/discovery.js +++ b/app-shell/src/discovery.js @@ -33,6 +33,7 @@ export function registerDiscovery(dispatch: Dispatch) { config = getConfig('discovery') store = new Store({ name: 'discovery', defaults: { services: [] } }) + let disableCache = config.disableCache client = createDiscoveryClient({ pollInterval: SLOW_POLL_INTERVAL_MS, @@ -51,6 +52,10 @@ export function registerDiscovery(dispatch: Dispatch) { client.setCandidates(['[fd00:0:cafe:fefe::1]'].concat(value)) ) + handleConfigChange('discovery.disableCache', value => { + disableCache = value + }) + app.once('will-quit', () => client.stop()) return function handleIncomingAction(action: Action) { @@ -73,7 +78,9 @@ export function registerDiscovery(dispatch: Dispatch) { } function handleServices() { - store.set('services', filterServicesToPersist(client.services)) + if (!disableCache) { + store.set('services', filterServicesToPersist(client.services)) + } dispatch({ type: 'discovery:UPDATE_LIST', payload: { robots: client.services }, diff --git a/app/src/components/NetworkSettingsCard/index.js b/app/src/components/NetworkSettingsCard/index.js index 91aa6b427cc..1d6e351f413 100644 --- a/app/src/components/NetworkSettingsCard/index.js +++ b/app/src/components/NetworkSettingsCard/index.js @@ -4,6 +4,7 @@ import * as React from 'react' import { Card } from '@opentrons/components' import { AddManualIp } from './AddManualIp' import { ClearDiscoveryCache } from './ClearDiscoveryCache' +import { CachingToggle } from './toggleDiscoveryCaching' // TODO(mc, 2020-04-27): i18n const NETWORK_SETTINGS = 'Network Settings' @@ -11,6 +12,7 @@ const NETWORK_SETTINGS = 'Network Settings' export const NetworkSettingsCard = () => ( + ) diff --git a/app/src/components/NetworkSettingsCard/toggleDiscoveryCaching.js b/app/src/components/NetworkSettingsCard/toggleDiscoveryCaching.js new file mode 100644 index 00000000000..a92d3f8d29a --- /dev/null +++ b/app/src/components/NetworkSettingsCard/toggleDiscoveryCaching.js @@ -0,0 +1,48 @@ +// @flow +import * as React from 'react' +import { connect } from 'react-redux' +import { LabeledToggle } from '@opentrons/components' +import type { State, Dispatch } from '../../types' +import { getConfig, toggleDiscoveryCache } from '../../config' + +type OP = {||} + +type SP = {| cacheDisabled: boolean |} + +type DP = {| toggleCacheDisable: () => mixed |} + +type Props = {| ...SP, ...DP |} + +export const CachingToggle = connect( + mapStateToProps, + mapDispatchToProps +)(CachingToggleComponent) + +function CachingToggleComponent(props: Props) { + return ( + +

+ Disable caching of discovered robots. The app will not remember + previously discovered robots. +

+ + ) +} + +function mapStateToProps(state: State): SP { + const config = getConfig(state) + + return { + cacheDisabled: config.discovery.disableCache, + } +} + +function mapDispatchToProps(dispatch: Dispatch): DP { + return { + toggleCacheDisable: () => dispatch(toggleDiscoveryCache()), + } +} diff --git a/app/src/config/actions.js b/app/src/config/actions.js index a19b5abfed7..b72669468bc 100644 --- a/app/src/config/actions.js +++ b/app/src/config/actions.js @@ -41,6 +41,12 @@ export function toggleDevInternalFlag(flag: DevInternalFlag): ThunkAction { } } +export function toggleDiscoveryCache(): ThunkAction { + return (dispatch, getState) => { + const cacheDisabled = getConfig(getState()).discovery.disableCache + return dispatch(updateConfig('discovery.disableCache', !cacheDisabled)) + } +} // TODO(mc, 2020-02-05): move to `discovery` module export function addManualIp(ip: string): ThunkAction { return (dispatch, getState) => { From 06a1baa294e08678b07725d9b7aaf23782f5ce4e Mon Sep 17 00:00:00 2001 From: Sanniti Date: Fri, 29 May 2020 12:24:50 -0400 Subject: [PATCH 02/10] address comments --- app-shell/src/discovery.js | 4 +- .../DisableDiscoveryCache.js | 30 ++++++++++++ .../components/NetworkSettingsCard/index.js | 4 +- .../toggleDiscoveryCaching.js | 48 ------------------- app/src/config/actions.js | 6 --- 5 files changed, 34 insertions(+), 58 deletions(-) create mode 100644 app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js delete mode 100644 app/src/components/NetworkSettingsCard/toggleDiscoveryCaching.js diff --git a/app-shell/src/discovery.js b/app-shell/src/discovery.js index 9a6fdeb7204..fc8543b32c7 100644 --- a/app-shell/src/discovery.js +++ b/app-shell/src/discovery.js @@ -39,7 +39,7 @@ export function registerDiscovery(dispatch: Dispatch) { pollInterval: SLOW_POLL_INTERVAL_MS, logger: log, candidates: ['[fd00:0:cafe:fefe::1]'].concat(config.candidates), - services: store.get('services'), + services: disableCache ? [] : store.get('services'), }) client @@ -55,7 +55,7 @@ export function registerDiscovery(dispatch: Dispatch) { handleConfigChange('discovery.disableCache', value => { disableCache = value }) - + app.once('will-quit', () => client.stop()) return function handleIncomingAction(action: Action) { diff --git a/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js b/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js new file mode 100644 index 00000000000..620ad8de3d0 --- /dev/null +++ b/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js @@ -0,0 +1,30 @@ +// @flow +import * as React from 'react' +import { useSelector, useDispatch } from 'react-redux' +import { LabeledToggle } from '@opentrons/components' +import type { State, Dispatch } from '../../types' +import { getConfig, updateConfig } from '../../config' + +export const DisableDiscoveryCache = () => { + const cacheDisabled = useSelector((state: State) => { + const config = getConfig(state) + return config.discovery.disableCache + }) + const dispatch = useDispatch() + return ( + { + dispatch(updateConfig('discovery.disableCache', !cacheDisabled)) + }} + > +

+ Disable caching of previously seen robots. Enabling this setting may + improve overall networking performance in environments with many OT-2s, + but may cause initial OT-2 discovery on app launch to be slower and more + susceptible to failures. +

+ + ) +} diff --git a/app/src/components/NetworkSettingsCard/index.js b/app/src/components/NetworkSettingsCard/index.js index 1d6e351f413..31d26770969 100644 --- a/app/src/components/NetworkSettingsCard/index.js +++ b/app/src/components/NetworkSettingsCard/index.js @@ -4,7 +4,7 @@ import * as React from 'react' import { Card } from '@opentrons/components' import { AddManualIp } from './AddManualIp' import { ClearDiscoveryCache } from './ClearDiscoveryCache' -import { CachingToggle } from './toggleDiscoveryCaching' +import { DisableDiscoveryCache } from './DisableDiscoveryCache' // TODO(mc, 2020-04-27): i18n const NETWORK_SETTINGS = 'Network Settings' @@ -12,7 +12,7 @@ const NETWORK_SETTINGS = 'Network Settings' export const NetworkSettingsCard = () => ( - + ) diff --git a/app/src/components/NetworkSettingsCard/toggleDiscoveryCaching.js b/app/src/components/NetworkSettingsCard/toggleDiscoveryCaching.js deleted file mode 100644 index a92d3f8d29a..00000000000 --- a/app/src/components/NetworkSettingsCard/toggleDiscoveryCaching.js +++ /dev/null @@ -1,48 +0,0 @@ -// @flow -import * as React from 'react' -import { connect } from 'react-redux' -import { LabeledToggle } from '@opentrons/components' -import type { State, Dispatch } from '../../types' -import { getConfig, toggleDiscoveryCache } from '../../config' - -type OP = {||} - -type SP = {| cacheDisabled: boolean |} - -type DP = {| toggleCacheDisable: () => mixed |} - -type Props = {| ...SP, ...DP |} - -export const CachingToggle = connect( - mapStateToProps, - mapDispatchToProps -)(CachingToggleComponent) - -function CachingToggleComponent(props: Props) { - return ( - -

- Disable caching of discovered robots. The app will not remember - previously discovered robots. -

- - ) -} - -function mapStateToProps(state: State): SP { - const config = getConfig(state) - - return { - cacheDisabled: config.discovery.disableCache, - } -} - -function mapDispatchToProps(dispatch: Dispatch): DP { - return { - toggleCacheDisable: () => dispatch(toggleDiscoveryCache()), - } -} diff --git a/app/src/config/actions.js b/app/src/config/actions.js index b72669468bc..a19b5abfed7 100644 --- a/app/src/config/actions.js +++ b/app/src/config/actions.js @@ -41,12 +41,6 @@ export function toggleDevInternalFlag(flag: DevInternalFlag): ThunkAction { } } -export function toggleDiscoveryCache(): ThunkAction { - return (dispatch, getState) => { - const cacheDisabled = getConfig(getState()).discovery.disableCache - return dispatch(updateConfig('discovery.disableCache', !cacheDisabled)) - } -} // TODO(mc, 2020-02-05): move to `discovery` module export function addManualIp(ip: string): ThunkAction { return (dispatch, getState) => { From 27ea3e0a6eb6d33d2fba53eed1e3b662b9bbfd47 Mon Sep 17 00:00:00 2001 From: Sanniti Date: Fri, 29 May 2020 16:33:52 -0400 Subject: [PATCH 03/10] clear cache when caching disabled --- app-shell/src/discovery.js | 3 +++ .../components/NetworkSettingsCard/DisableDiscoveryCache.js | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app-shell/src/discovery.js b/app-shell/src/discovery.js index fc8543b32c7..c4c03fdb2c3 100644 --- a/app-shell/src/discovery.js +++ b/app-shell/src/discovery.js @@ -53,6 +53,9 @@ export function registerDiscovery(dispatch: Dispatch) { ) handleConfigChange('discovery.disableCache', value => { + if (value === true) { + clearCache() + } disableCache = value }) diff --git a/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js b/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js index 620ad8de3d0..48cb740080d 100644 --- a/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js +++ b/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js @@ -13,7 +13,7 @@ export const DisableDiscoveryCache = () => { const dispatch = useDispatch() return ( { dispatch(updateConfig('discovery.disableCache', !cacheDisabled)) From 1ced5dc49a506082835691e1921079aa01ba0494 Mon Sep 17 00:00:00 2001 From: Sanniti Date: Tue, 2 Jun 2020 10:38:44 -0400 Subject: [PATCH 04/10] added component tests --- .../DisableDiscoveryCache.js | 5 +- .../__tests__/DisableDiscoveryCache.test.js | 84 +++++++++++++++++++ .../__tests__/NetworkSettingsCard.test.js | 6 ++ 3 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js diff --git a/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js b/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js index 48cb740080d..334bdc068a1 100644 --- a/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js +++ b/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js @@ -13,13 +13,14 @@ export const DisableDiscoveryCache = () => { const dispatch = useDispatch() return ( { dispatch(updateConfig('discovery.disableCache', !cacheDisabled)) - }} + }} >

+ NOTE: This will clear cached robots when switched ON Disable caching of previously seen robots. Enabling this setting may improve overall networking performance in environments with many OT-2s, but may cause initial OT-2 discovery on app launch to be slower and more diff --git a/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js b/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js new file mode 100644 index 00000000000..e58bc0aa425 --- /dev/null +++ b/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js @@ -0,0 +1,84 @@ +// @flow +import * as React from 'react' +import { Provider } from 'react-redux' +import { mount } from 'enzyme' +import noop from 'lodash/noop' + +import * as AllConfig from '../../../config' +import { LabeledToggle } from '@opentrons/components' +import { DisableDiscoveryCache } from '../DisableDiscoveryCache' + +import type { State } from '../../../types' +import type { Config } from '../../../config/types' + +jest.mock('../../../config/selectors') + +const MOCK_STATE: State = ({ mockState: true }: any) + +const getConfig: JestMockFn<[State], Config> = AllConfig.getConfig + +function stubSelector(mock: JestMockFn<[State], R>, rVal: R) { + mock.mockImplementation(state => { + expect(state).toBe(MOCK_STATE) + return rVal + }) +} + +describe('DisableDiscoveryCache', () => { + const dispatch = jest.fn() + const MOCK_STORE = { + dispatch: dispatch, + subscribe: noop, + getState: () => MOCK_STATE, + } + + const render = () => { + return mount(, { + wrappingComponent: Provider, + wrappingComponentProps: { store: MOCK_STORE }, + }) + } + + beforeEach(() => { + stubSelector(getConfig, { + discovery: { + candidates: [], + disableCache: false, + }, + }) + }) + + afterEach(() => { + jest.resetAllMocks() + }) + + it('renders a labelled toggle component', () => { + const wrapper = render() + const theToggle = wrapper.find(LabeledToggle) + expect(theToggle.prop('label')).toBe('Disable robot caching') + expect(theToggle.prop('toggledOn')).toBe(false) + }) + + it('updates the toggle on change to disableCache', () => { + stubSelector(getConfig, { + discovery: { + candidates: [], + disableCache: true, + }, + }) + const wrapper = render() + const theToggle = wrapper.find(LabeledToggle) + expect(theToggle.prop('toggledOn')).toBe(true) + }) + + it('dispatches config update on toggle', () => { + const wrapper = render() + const theToggle = wrapper.find(LabeledToggle) + theToggle.prop('onClick')() + expect(dispatch).toHaveBeenCalledWith({ + type: 'config:UPDATE', + payload: { path: 'discovery.disableCache', value: true }, + meta: { shell: true }, + }) + }) +}) diff --git a/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js b/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js index ea0b540bcff..97329b02e51 100644 --- a/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js +++ b/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js @@ -6,6 +6,7 @@ import { Card } from '@opentrons/components' import { AddManualIp } from '../AddManualIp' import { NetworkSettingsCard } from '..' import { ClearDiscoveryCache } from '../ClearDiscoveryCache' +import { DisableDiscoveryCache } from '../DisableDiscoveryCache' describe('NetworkSettingsCard', () => { it('should render a card with the proper title', () => { @@ -22,4 +23,9 @@ describe('NetworkSettingsCard', () => { const wrapper = shallow() expect(wrapper.exists(ClearDiscoveryCache)).toBe(true) }) + + it('should render a DisableDiscoveryCache component', () => { + const wrapper = shallow() + expect(wrapper.exists(DisableDiscoveryCache)).toBe(true) + }) }) From de13a36363f4797db379de9400a44dcac5af7902 Mon Sep 17 00:00:00 2001 From: Sanniti Date: Tue, 2 Jun 2020 10:42:04 -0400 Subject: [PATCH 05/10] better test description --- .../NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js b/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js index e58bc0aa425..4ef5ab0b2c7 100644 --- a/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js +++ b/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js @@ -59,7 +59,7 @@ describe('DisableDiscoveryCache', () => { expect(theToggle.prop('toggledOn')).toBe(false) }) - it('updates the toggle on change to disableCache', () => { + it('updates the toggle status according to disableCache config', () => { stubSelector(getConfig, { discovery: { candidates: [], From 3b7b5116b1d2a50d54e6106e424e71be163c37ae Mon Sep 17 00:00:00 2001 From: Sanniti Date: Tue, 2 Jun 2020 10:52:58 -0400 Subject: [PATCH 06/10] added component description test --- .../__tests__/DisableDiscoveryCache.test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js b/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js index 4ef5ab0b2c7..6ff8a1971b8 100644 --- a/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js +++ b/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js @@ -59,6 +59,13 @@ describe('DisableDiscoveryCache', () => { expect(theToggle.prop('toggledOn')).toBe(false) }) + it('renders description of the toggle component', () => { + const wrapper = render() + expect(wrapper.children().html()).toMatch( + /Disable caching of previously seen robots./ + ) + }) + it('updates the toggle status according to disableCache config', () => { stubSelector(getConfig, { discovery: { From 7c060925c68081310cb13d8242c6696edd7e36f6 Mon Sep 17 00:00:00 2001 From: Sanniti Date: Tue, 2 Jun 2020 13:25:21 -0400 Subject: [PATCH 07/10] address new comments --- .../DisableDiscoveryCache.js | 4 ++- .../__tests__/DisableDiscoveryCache.test.js | 30 +++++++++---------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js b/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js index 334bdc068a1..225d007b510 100644 --- a/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js +++ b/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js @@ -20,7 +20,9 @@ export const DisableDiscoveryCache = () => { }} >

- NOTE: This will clear cached robots when switched ON + NOTE: This will clear cached robots when switched ON. +

+

Disable caching of previously seen robots. Enabling this setting may improve overall networking performance in environments with many OT-2s, but may cause initial OT-2 discovery on app launch to be slower and more diff --git a/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js b/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js index 6ff8a1971b8..cdaadc4a3fc 100644 --- a/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js +++ b/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js @@ -41,10 +41,7 @@ describe('DisableDiscoveryCache', () => { beforeEach(() => { stubSelector(getConfig, { - discovery: { - candidates: [], - disableCache: false, - }, + discovery: { candidates: [], disableCache: false }, }) }) @@ -68,24 +65,27 @@ describe('DisableDiscoveryCache', () => { it('updates the toggle status according to disableCache config', () => { stubSelector(getConfig, { - discovery: { - candidates: [], - disableCache: true, - }, + discovery: { candidates: [], disableCache: true }, }) const wrapper = render() - const theToggle = wrapper.find(LabeledToggle) - expect(theToggle.prop('toggledOn')).toBe(true) + expect(wrapper.find(LabeledToggle).prop('toggledOn')).toBe(true) + + // toggle switches value + stubSelector(getConfig, { + discovery: { candidates: [], disableCache: false }, + }) + + // trigger a re-render + wrapper.setProps({}) + expect(wrapper.find(LabeledToggle).prop('toggledOn')).toBe(false) }) it('dispatches config update on toggle', () => { const wrapper = render() const theToggle = wrapper.find(LabeledToggle) theToggle.prop('onClick')() - expect(dispatch).toHaveBeenCalledWith({ - type: 'config:UPDATE', - payload: { path: 'discovery.disableCache', value: true }, - meta: { shell: true }, - }) + expect(dispatch).toHaveBeenCalledWith( + AllConfig.updateConfig('discovery.disableCache', true) + ) }) }) From 9dc66d0a687a6bf29150fa1b3ccd3e541d1d5ad2 Mon Sep 17 00:00:00 2001 From: Sanniti Date: Wed, 3 Jun 2020 10:33:54 -0400 Subject: [PATCH 08/10] added discovery tests --- app-shell/src/__tests__/discovery.test.js | 59 ++++++++++++++++++- .../__tests__/NetworkSettingsCard.test.js | 2 +- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/app-shell/src/__tests__/discovery.test.js b/app-shell/src/__tests__/discovery.test.js index abfb21e9c9e..bfc16a452b8 100644 --- a/app-shell/src/__tests__/discovery.test.js +++ b/app-shell/src/__tests__/discovery.test.js @@ -4,7 +4,7 @@ import { app } from 'electron' import Store from 'electron-store' import { createDiscoveryClient } from '@opentrons/discovery-client' import { registerDiscovery } from '../discovery' -import { getConfig, getOverrides } from '../config' +import { getConfig, getOverrides, handleConfigChange } from '../config' jest.mock('electron') jest.mock('electron-store') @@ -12,6 +12,9 @@ jest.mock('@opentrons/discovery-client') jest.mock('../log') jest.mock('../config') +const _handleConfigChange: JestMockFn<[string, (any, any) => mixed], mixed> = + handleConfigChange + describe('app-shell/discovery', () => { let dispatch let mockClient @@ -259,4 +262,58 @@ describe('app-shell/discovery', () => { payload: { robots: [] }, }) }) + + it('does not update services from store when caching disabled', () => { + // cache has been disabled + getConfig.mockReturnValue({ + enabled: true, + candidates: [], + disableCache: true, + }) + // discovery.json contains 1 entry + Store.__store.get.mockImplementation(key => { + if (key === 'services') return [{ name: 'foo' }] + return null + }) + + registerDiscovery(dispatch) + + // should not contain above entry + expect(createDiscoveryClient).toHaveBeenCalledWith( + expect.objectContaining({ + services: [], + }) + ) + console.log(Store) + }) + + it('clears cache & suspends caching when caching changes to disabled', () => { + // Cache enabled initially + getConfig.mockReturnValue({ + enabled: true, + candidates: [], + disableCache: false, + }) + // discovery.json contains 1 entry + Store.__store.get.mockImplementation(key => { + if (key === 'services') return [{ name: 'foo' }] + return null + }) + + registerDiscovery(dispatch) + + // the 'discovery.disableCache' change handler + const changeHandler = _handleConfigChange.mock.calls[1][1] + const disableCache = true + changeHandler(disableCache) + + expect(Store.__store.set).toHaveBeenCalledWith('services', []) + + // new services discovered + mockClient.services = [{ name: 'foo' }, { name: 'bar' }] + mockClient.emit('service') + + // but discovery.json should not update + expect(Store.__store.set).toHaveBeenLastCalledWith('services', []) + }) }) diff --git a/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js b/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js index 97329b02e51..d27e1aa32b3 100644 --- a/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js +++ b/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js @@ -25,7 +25,7 @@ describe('NetworkSettingsCard', () => { }) it('should render a DisableDiscoveryCache component', () => { - const wrapper = shallow() + const wrapper = shallow() expect(wrapper.exists(DisableDiscoveryCache)).toBe(true) }) }) From 264131a951571db42529da9e7077bc57d985638d Mon Sep 17 00:00:00 2001 From: Sanniti Date: Wed, 3 Jun 2020 11:46:45 -0400 Subject: [PATCH 09/10] fix linter errors --- app-shell/src/__tests__/discovery.test.js | 7 +++++-- .../NetworkSettingsCard/DisableDiscoveryCache.js | 4 +--- .../__tests__/DisableDiscoveryCache.test.js | 2 +- .../__tests__/NetworkSettingsCard.test.js | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app-shell/src/__tests__/discovery.test.js b/app-shell/src/__tests__/discovery.test.js index bfc16a452b8..6aa440a1fdd 100644 --- a/app-shell/src/__tests__/discovery.test.js +++ b/app-shell/src/__tests__/discovery.test.js @@ -1,3 +1,4 @@ +// @flow // tests for the app-shell's discovery module import EventEmitter from 'events' import { app } from 'electron' @@ -12,8 +13,10 @@ jest.mock('@opentrons/discovery-client') jest.mock('../log') jest.mock('../config') -const _handleConfigChange: JestMockFn<[string, (any, any) => mixed], mixed> = - handleConfigChange +const _handleConfigChange: JestMockFn< + [string, (any, any) => mixed], + mixed +> = handleConfigChange describe('app-shell/discovery', () => { let dispatch diff --git a/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js b/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js index 225d007b510..7438883ce5c 100644 --- a/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js +++ b/app/src/components/NetworkSettingsCard/DisableDiscoveryCache.js @@ -19,9 +19,7 @@ export const DisableDiscoveryCache = () => { dispatch(updateConfig('discovery.disableCache', !cacheDisabled)) }} > -

- NOTE: This will clear cached robots when switched ON. -

+

NOTE: This will clear cached robots when switched ON.

Disable caching of previously seen robots. Enabling this setting may improve overall networking performance in environments with many OT-2s, diff --git a/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js b/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js index cdaadc4a3fc..248960a04af 100644 --- a/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js +++ b/app/src/components/NetworkSettingsCard/__tests__/DisableDiscoveryCache.test.js @@ -15,7 +15,7 @@ jest.mock('../../../config/selectors') const MOCK_STATE: State = ({ mockState: true }: any) -const getConfig: JestMockFn<[State], Config> = AllConfig.getConfig +const getConfig: JestMockFn<[State], $Shape> = AllConfig.getConfig function stubSelector(mock: JestMockFn<[State], R>, rVal: R) { mock.mockImplementation(state => { diff --git a/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js b/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js index d27e1aa32b3..9b7f3f1f755 100644 --- a/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js +++ b/app/src/components/NetworkSettingsCard/__tests__/NetworkSettingsCard.test.js @@ -27,5 +27,5 @@ describe('NetworkSettingsCard', () => { it('should render a DisableDiscoveryCache component', () => { const wrapper = shallow() expect(wrapper.exists(DisableDiscoveryCache)).toBe(true) - }) + }) }) From efc3f720b4e41c1c5244471b4dd4487e102b8450 Mon Sep 17 00:00:00 2001 From: Sanniti Date: Wed, 3 Jun 2020 12:14:32 -0400 Subject: [PATCH 10/10] suppress flow errors for now --- app-shell/src/__tests__/discovery.test.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/app-shell/src/__tests__/discovery.test.js b/app-shell/src/__tests__/discovery.test.js index 6aa440a1fdd..99958c3c3c5 100644 --- a/app-shell/src/__tests__/discovery.test.js +++ b/app-shell/src/__tests__/discovery.test.js @@ -1,4 +1,3 @@ -// @flow // tests for the app-shell's discovery module import EventEmitter from 'events' import { app } from 'electron' @@ -13,11 +12,6 @@ jest.mock('@opentrons/discovery-client') jest.mock('../log') jest.mock('../config') -const _handleConfigChange: JestMockFn< - [string, (any, any) => mixed], - mixed -> = handleConfigChange - describe('app-shell/discovery', () => { let dispatch let mockClient @@ -269,7 +263,6 @@ describe('app-shell/discovery', () => { it('does not update services from store when caching disabled', () => { // cache has been disabled getConfig.mockReturnValue({ - enabled: true, candidates: [], disableCache: true, }) @@ -287,13 +280,11 @@ describe('app-shell/discovery', () => { services: [], }) ) - console.log(Store) }) it('clears cache & suspends caching when caching changes to disabled', () => { // Cache enabled initially getConfig.mockReturnValue({ - enabled: true, candidates: [], disableCache: false, }) @@ -306,7 +297,7 @@ describe('app-shell/discovery', () => { registerDiscovery(dispatch) // the 'discovery.disableCache' change handler - const changeHandler = _handleConfigChange.mock.calls[1][1] + const changeHandler = handleConfigChange.mock.calls[1][1] const disableCache = true changeHandler(disableCache)