Make ms-vscode.live-server an optional dependency #480
Conversation
test/runTest.ts
Outdated
| // Add the dependent extension for test coverage preview functionality | ||
| const vscodeExecutablePath = await downloadAndUnzipVSCode("insiders"); | ||
| const [cliPath, ...args] = resolveCliArgsFromVSCodeExecutablePath(vscodeExecutablePath); | ||
| // Ensure live-server is not installed |
There was a problem hiding this comment.
I'm not much of a fan of this as it means that we end up running all the tests twice, so this roughly doubles the amount of time it takes to run the tests, but wasn't able to find a better way of running a test without the live server extension installed...
There was a problem hiding this comment.
If you wrap const livePreview = extensions.getExtension("ms-vscode.live-server"); in a function, you can stub that function in a couple of tests and get rid of all of this.
There was a problem hiding this comment.
or leave it installed and only stub it being missing in one test
There was a problem hiding this comment.
That's a much better idea! Implemented. I had to partially merge in your #482 branch in order to implement this
|
|
||
| // Useful for debugging failing dependant extension installs | ||
| console.info(output); |
There was a problem hiding this comment.
The runVSCodeCommand function will throw if calling VSCode fails, so this will be caught by the catch block now
There was a problem hiding this comment.
Excellent! This is a super nice cleanup 🦾 .
|
@Gerrit0 Thanks for the PR and thanks @mattseddon for reviewing it, looking great so far. I merged the other PR with the test changes / fixups, so I have rerun the CI action for yeah Gerrit. |
| await window.showErrorMessage("Live Preview extension not installed", { | ||
| modal: true, | ||
| detail: "The ms-vscode.live-server extension must be installed to preview the coverage report." | ||
| }, "Ok"); |
There was a problem hiding this comment.
Super minor thought: I have never had to do multiple languages for the extension, do we need to support that for the errors or is it more of a "nice to have"?
There was a problem hiding this comment.
I considered that, but decided not to open that can of worms at this point given that there are other messages which contain hardcoded English text:
- https://github.com/ryanluker/vscode-coverage-gutters/blob/master/src/files/filesloader.ts#L22
- https://github.com/ryanluker/vscode-coverage-gutters/blob/master/src/files/filesloader.ts#L108
- https://github.com/ryanluker/vscode-coverage-gutters/blob/master/src/extension/gutters.ts#L38
- https://github.com/ryanluker/vscode-coverage-gutters/blob/master/src/extension/statusbartoggler.ts#L21-L22
Removes hard dependency on live-server extension, replacing it with an error message if the preview command is used when not installed.
Resolves #392