feat: refactored devices command and added health and status flags. #369
Conversation
🦋 Changeset detectedLatest commit: 61067b1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
bflorian
force-pushed
the
device-health-status
branch
from
8913398 to
a48f48d
Compare
| const device = await this.client.devices.get(id, deviceGetOptions) | ||
| // Note -- we have to do this explicitly because the API does not honor the includeHealth parameter | ||
| // for individual devices | ||
| if (this.flags.health) { |
There was a problem hiding this comment.
This could be refactored to be a parallel call since we aren't using the device to get the health state.
bflorian
changed the title
bflorian
force-pushed
the
device-health-status
branch
from
a48f48d to
61067b1
Compare
| const tablePushMock: jest.Mock<number, [(string | undefined)[]]> = jest.fn() | ||
| const tableToStringMock = jest.fn() | ||
| const tableMock = { | ||
| push: tablePushMock, | ||
| toString: tableToStringMock, | ||
| } as unknown as Table | ||
| const newOutputTableMock = jest.fn().mockReturnValue(tableMock) | ||
|
|
||
| const tableGeneratorMock: TableGenerator = { | ||
| newOutputTable: newOutputTableMock, | ||
| buildTableFromItem: jest.fn(), | ||
| buildTableFromList: jest.fn(), | ||
| } as TableGenerator |
There was a problem hiding this comment.
I see these mocks repeated throughout several tests. We should extract them out to a common scope for reuse.
There was a problem hiding this comment.
Will refactor these in next PR, which is in progress
| const tablePushMock: jest.Mock<number, [(string | undefined)[]]> = jest.fn() | ||
| const tableToStringMock = jest.fn() |
There was a problem hiding this comment.
These jest.fn() as well as the TableGenerator mock functions below can be specified with jest.mocked(TableAdapter.prototype.push)
jest.mocked(DefaultTableGenerator.prototype.newOutputTable)
etc. which should give us more type safety.
| const deviceId = 'device-id' | ||
| const getSpy = jest.spyOn(DevicesEndpoint.prototype, 'get').mockImplementation() |
There was a problem hiding this comment.
These should be declared at the describe level to reuse throughout the tests.
| "@smartthings/cli": patch | ||
| --- | ||
|
|
||
| feat: Refactored devices command and added health and status flags. |
There was a problem hiding this comment.
This is minor these are used for release notes and it might be more clear to just say something like added health and status flags to devices command (no need for feat: here either).
I wonder if we shouldn't come up with some sort of a little style guide for these.
| }) | ||
|
|
||
| it('uses devices.get to get device', async () => { | ||
| await expect(DevicesCommand.run(['--capability', 'cmd-line-capability'])).resolves.not.toThrow() |
|
|
||
| export const buildTableOutput = (tableGenerator: TableGenerator, device: Device & { profileId?: string; healthState?: DeviceHealth }): string => { | ||
| const table = tableGenerator.newOutputTable() | ||
| table.push(['Label', device.label]) |
There was a problem hiding this comment.
Moving this is such a minor thing but it definitely belongs where you put it. 👍
| return output | ||
| } | ||
|
|
||
| export const buildEmbeddedStatusTableOutput = (tableGenerator: TableGenerator, data: Device): string => { |
There was a problem hiding this comment.
I might play with trying to merge these two functions after you merge this if that's okay. (I'm thinking maybe a single generic function and then two glue functions that call it, assuming I can figure out how to make the types work.)
3 participants
Refactored devices command to move utility functions into a separate module and added flags to include device health and device status information to device list and get commands.
Checklist
npm run lintproduces no warnings/errors)