Add tests for column depth configuration #2471
Conversation
julieg18
changed the title
|
Code Climate has analyzed commit 3e27203 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 97.0% (0.0% change). View more on Code Climate. |
| jest.mock('../../../vscode/config') | ||
|
|
||
| const mockedGetConfigValue = jest.mocked(getConfigValue) | ||
| mockedGetConfigValue.mockImplementation(() => 5) |
There was a problem hiding this comment.
[I] The usual pattern is to use beforeEach with resetAllMocks to keep tests independent
beforeEach(() => {
jest.resetAllMocks()
})
I thin you could use mockReturnValueOnce instead of mockImplementation as well. Final code would probably be
beforeEach(() => {
jest.resetAllMocks()
mockedGetConfigValue.mockReturnValueOnce(5)
})
for this particular file as we are not varying the option.
| workspace.getConfiguration().get(key, '') as unknown as T | ||
| export const getConfigValue = <T = string, D = string>( | ||
| key: ConfigKey, | ||
| defaultValue?: D | T |
There was a problem hiding this comment.
does this translates to defaultValue?: string ... 😕
There was a problem hiding this comment.
I think this could be
export const getConfigValue = <T = string>(
key: ConfigKey,
defaultValue: T | undefined
): T =>
workspace.getConfiguration().get(key, defaultValue ?? '') as unknown as T
| jest.mock('../../vscode/config') | ||
|
|
||
| const mockedGetConfigValue = jest.mocked(getConfigValue) | ||
| mockedGetConfigValue.mockImplementation(() => 5) |
There was a problem hiding this comment.
[I] Can also use a beforeEach this time I would but the mockReturnValueOnce calls inside of the tests as the value is being varied throughout the file
mattseddon
left a comment
mattseddon
left a comment
There was a problem hiding this comment.
This was meant to be an approval
4 participants
Followup to #2436 comment