Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(app): Refactor /health modules to use generic API actions #1826

Merged
merged 1 commit into from
Jul 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/src/components/LostConnectionAlert/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
actions as robotActions
} from '../../robot'

import {makeGetHealthCheckOk} from '../../http-api-client'
import {makeGetHealthCheckOk} from '../../health-check'

import {AlertModal} from '@opentrons/components'
import ModalCopy from './ModalCopy'
Expand Down
4 changes: 2 additions & 2 deletions app/src/config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// config redux module
import {setIn} from '@thi.ng/paths'
import {getShellConfig} from '../shell'
import type {State, Action} from '../types'
import type {State, Action, ThunkAction} from '../types'
import type {LogLevel} from '../logger'

type UrlProtocol = 'file:' | 'http:'
Expand Down Expand Up @@ -95,7 +95,7 @@ export function getModulesOn (state: State): boolean {
return state.config.modules
}

export function toggleDevTools (): Action {
export function toggleDevTools (): ThunkAction {
return (dispatch, getState) => {
const devToolsOn = getDevToolsOn(getState())
return dispatch(updateConfig('devtools', !devToolsOn))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
// health check tests
import {actions as robotActions} from '../../robot'
import {__mockThunk, fetchHealth} from '../health'
import {__mockThunk, fetchHealth} from '../../http-api-client/health'

import {
reducer,
startHealthCheck,
stopHealthCheck,
setHealthCheckId,
clearHealthCheckId,
resetHealthCheck,
healthCheckReducer,
healthCheckMiddleware,
makeGetHealthCheckOk
} from '..'

jest.mock('../health')
jest.mock('../../http-api-client/health')

const name = 'opentrons-dev'
const robot = {name, ip: '1.2.3.4', port: '1234'}
Expand All @@ -29,12 +29,10 @@ describe('health check', () => {
connectRequest: {name},
discoveredByName: {[name]: robot}}
},
api: {
healthCheck: {
alreadyRunning: {id: 1, missed: 0},
alreadyMissed: {id: 2, missed: 2},
[name]: {id: null, missed: 1}
}
healthCheck: {
alreadyRunning: {id: 1, missed: 0},
alreadyMissed: {id: 2, missed: 2},
[name]: {id: null, missed: 1}
}
}
})
Expand Down Expand Up @@ -169,27 +167,27 @@ describe('health check', () => {
.toHaveBeenCalledWith(resetHealthCheck(expectedRobot))
})

test('HEALTH_FAILURE sends STOP_HEALTH_CHECK if over threshold', () => {
test('api:FAILURE sends STOP_HEALTH_CHECK if over threshold', () => {
const robot = {name: 'alreadyMissed'}
invoke({type: 'api:HEALTH_FAILURE', payload: {robot}})
invoke({type: 'api:FAILURE', payload: {robot, path: 'health'}})
expect(store.dispatch).toHaveBeenCalledWith(stopHealthCheck(robot))
})

test('HEALTH_FAILURE noops if under threshold', () => {
test('api:FAILURE noops if under threshold', () => {
const robot = {name: 'alreadyRunning'}
invoke({type: 'api:HEALTH_FAILURE', payload: {robot}})
invoke({type: 'api:FAILURE', payload: {robot, path: 'health'}})
expect(store.dispatch).not.toHaveBeenCalledWith(stopHealthCheck(robot))
})

test('HEALTH_FAILURE noops if not running', () => {
state.api.healthCheck[name].missed = 2
invoke({type: 'api:HEALTH_FAILURE', payload: {robot}})
test('api:FAILURE noops if not running', () => {
state.healthCheck[name].missed = 2
invoke({type: 'api:FAILURE', payload: {robot, path: 'health'}})
expect(store.dispatch).not.toHaveBeenCalledWith(stopHealthCheck(robot))
})
})

describe('reducer', () => {
const reduce = (action) => reducer(state.api, action).healthCheck
const reduce = (action) => healthCheckReducer(state.healthCheck, action)

test('RESET_HEALTH_CHECK resets interval ID and count', () => {
const action = resetHealthCheck({name: 'alreadyMissed'})
Expand Down Expand Up @@ -218,37 +216,37 @@ describe('health check', () => {
})
})

test('HEALTH_SUCCESS resets missed to 0', () => {
test('api:SUCCESS resets missed to 0', () => {
const robot = {name: 'alreadyMissed'}
const action = {type: 'api:HEALTH_SUCCESS', payload: {robot}}
const action = {type: 'api:SUCCESS', payload: {robot, path: 'health'}}
expect(reduce(action)).toEqual({
alreadyRunning: {id: 1, missed: 0},
alreadyMissed: {id: 2, missed: 0},
[name]: {id: null, missed: 1}
})
})

test('HEALTH_SUCCESS noops if not running', () => {
const action = {type: 'api:HEALTH_SUCCESS', payload: {robot}}
test('api:SUCCESS noops if not running', () => {
const action = {type: 'api:SUCCESS', payload: {robot, path: 'health'}}
expect(reduce(action)).toEqual({
alreadyRunning: {id: 1, missed: 0},
alreadyMissed: {id: 2, missed: 2},
[name]: {id: null, missed: 1}
})
})

test('HEALTH_FAILURE increases missed by 1', () => {
test('api:FAILURE increases missed by 1', () => {
const robot = {name: 'alreadyRunning'}
const action = {type: 'api:HEALTH_FAILURE', payload: {robot}}
const action = {type: 'api:FAILURE', payload: {robot, path: 'health'}}
expect(reduce(action)).toEqual({
alreadyRunning: {id: 1, missed: 1},
alreadyMissed: {id: 2, missed: 2},
[name]: {id: null, missed: 1}
})
})

test('HEALTH_FAILURE noops if not running', () => {
const action = {type: 'api:HEALTH_FAILURE', payload: {robot}}
test('api:FAILURE noops if not running', () => {
const action = {type: 'api:FAILURE', payload: {robot, path: 'health'}}
expect(reduce(action)).toEqual({
alreadyRunning: {id: 1, missed: 0},
alreadyMissed: {id: 2, missed: 2},
Expand All @@ -259,7 +257,7 @@ describe('health check', () => {

describe('selectors', () => {
test('makeGetHealthCheckOk', () => {
state.api.healthCheck[name].missed = 2
state.healthCheck[name].missed = 2
const getHealthCheckOk = makeGetHealthCheckOk()

// health check fails if id is cleared and missed threshold exceeded
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
getConnectedRobotName
} from '../robot/selectors'

import {fetchHealth} from './health'
import {fetchHealth} from '../http-api-client'

// since middleware triggers before actions are reduced, health check failure
// is triggered after CHECK_THRESHOLD + 1 missed polls
Expand Down Expand Up @@ -115,8 +115,10 @@ export const healthCheckMiddleware: Middleware =
stopChecking(store, action.payload.robot)
break

case 'api:HEALTH_FAILURE':
handleHealthFailure(store, action.payload.robot)
case 'api:FAILURE':
if (action.payload.path === 'health') {
handleHealthFailure(store, action.payload.robot)
}
break

case 'robot:CONNECT_RESPONSE':
Expand All @@ -143,36 +145,38 @@ export function healthCheckReducer (
): HealthCheckState {
if (!state) return {}

let name
let stateByName

switch (action.type) {
case 'api:RESET_HEALTH_CHECK':
name = action.payload.robot.name
case 'api:RESET_HEALTH_CHECK': {
const name = action.payload.robot.name
return {...state, [name]: INITIAL_STATE_BY_NAME}
}

case 'api:SET_HEALTH_CHECK_ID':
name = action.payload.robot.name
case 'api:SET_HEALTH_CHECK_ID': {
const name = action.payload.robot.name
return {...state, [name]: {id: action.payload.id, missed: 0}}
}

case 'api:CLEAR_HEALTH_CHECK_ID':
name = action.payload.robot.name
stateByName = state[name]
case 'api:CLEAR_HEALTH_CHECK_ID': {
const name = action.payload.robot.name
const stateByName = state[name]
return {...state, [name]: {...stateByName, id: null}}
}

case 'api:HEALTH_SUCCESS':
name = action.payload.robot.name
stateByName = state[name]
return stateByName && stateByName.id
case 'api:SUCCESS': {
const name = action.payload.robot.name
const stateByName = state[name] || {}
return action.payload.path === 'health' && stateByName.id
? {...state, [name]: {...stateByName, missed: 0}}
: state
}

case 'api:HEALTH_FAILURE':
name = action.payload.robot.name
stateByName = state[name]
return stateByName && stateByName.id
case 'api:FAILURE': {
const name = action.payload.robot.name
const stateByName = state[name] || {}
return action.payload.path === 'health' && stateByName.id
? {...state, [name]: {...stateByName, missed: stateByName.missed + 1}}
: state
}
}

return state
Expand Down Expand Up @@ -225,5 +229,5 @@ function selectRobotHealthCheck (
state: State,
props: BaseRobot
): ?RobotHealthCheck {
return state.api.healthCheck[props.name]
return state.healthCheck[props.name]
}
Loading