Add unit tests for lldb.ts #1110
Conversation
Cover all the error return cases. Issue: swiftlang#1023
michael-weng
requested review from
0xTim,
adam-fowler and
award999
as code owners
| } from "../../MockUtils"; | ||
| import { SwiftToolchain } from "../../../src/toolchain/toolchain"; | ||
| import { WorkspaceContext } from "../../../src/WorkspaceContext"; | ||
| import sinon = require("sinon"); |
There was a problem hiding this comment.
import * as sinon from 'sinon'
| import { WorkspaceContext } from "../../../src/WorkspaceContext"; | ||
| import sinon = require("sinon"); | ||
|
|
||
| suite("getLLDBLibPath Tests", () => { |
There was a problem hiding this comment.
since there is other functions in the module, lets follow the naming matt outlined https://github.com/swiftlang/vscode-swift/blob/main/docs/contributor/writing-tests-for-vscode-swift.md#writing-unit-tests
There was a problem hiding this comment.
Oh I see below you have one suite for each function, it is a common mocha pattern to have 1 top level suite in a module so lldb Unit Test Suite that then has nested suites for each function
| }); | ||
|
|
||
| teardown(() => { | ||
| mockFindLibLLDB.reset(); |
There was a problem hiding this comment.
Is setup is recreating each test, shouldn't have to reset
| let result = await lldb.getLLDBLibPath(instance(mockToolchain)); | ||
| expect(result.failure).to.not.equal(undefined); | ||
|
|
||
| mockFindLibLLDB.reset(); |
There was a problem hiding this comment.
we shouldn't need to reset, you can do something like mockFindLibLLDB.onFirstCall().resolves(...)
- make suite nested to adhere to one suite per module convention - clean up some teardown code and remove redundancy in test - fix date in copyright - change to typescript import
|
|
||
| expect(result).to.equal("/path/hint"); | ||
| }); | ||
| // NB(separate itest): contract test with toolchains of various platforms |
There was a problem hiding this comment.
question: what does the NB stand for?
There was a problem hiding this comment.
| expect(result.failure).to.have.property("message", "Failed to get LLDB"); | ||
| }); | ||
|
|
||
| test("should return failure when execFile throws an error on windows, specific behaviour: return success and failure both undefined", async function () { |
There was a problem hiding this comment.
this is wordy, not sure intent, and seems like we're skipping on windows not should return failure when execFile throws an error on windows
we should make intent more clear
3 participants
Cover all the error return cases.
Issue: #1023