ryanluker / vscode-coverage-gutters

Display test coverage generated by lcov and xml - works with many languages
https://marketplace.visualstudio.com/items?itemName=ryanluker.vscode-coverage-gutters
MIT License
460 stars 88 forks source link

Make manualCoverageFilePaths watchable #270

Closed amatosov-rbx closed 4 years ago

amatosov-rbx commented 4 years ago

Rational

I would like to use manualCoverageFilePaths with a set of explicit lcov files and avoid glob lookups, in order to optimize coverage-gutter on big workspaces.

createFileSystemWatcher works well with the glob pattern which contains files inside and outside of the workspace and successfully watches files that are inside workspace folders

Changes

Update listenToFileSystem to listen for manualCoverageFilePaths if they are provided

Tested on

Version: 1.47.1
Commit: 485c41f9460bdb830c4da12c102daff275415b53
Date: 2020-07-13T22:43:02.577Z
Electron: 7.3.2
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Darwin x64 19.6.0
ryanluker commented 4 years ago

@amatosov-rbx Thanks for the pull request! Yah this makes sense to do as the coverage service already uses this manual config setting anyways and should respect that when listening to the file system too, good call! My only ask would be to add a small unit test to the test suite please 😁 and then this can be merged in against the 2.6.0 release. If you are feeling really adventurous you could even add it as an integration test so that way we know this works for windows, linux and macs 🤔 .

amatosov-rbx commented 4 years ago

You are welcome @ryanluker. I have added couple unit tests for the CoverageService that verify glob creation for manual coverage files and regular use cases. I am not sure how to implement integration tests for watch tho. Triggering watch update requires modifying files during the test execution, but tests run against the example directory. I also noticed that there are no existing tests doing that type of checks. What would be the best way to implement this sort of watch tests?

ryanluker commented 4 years ago

@amatosov-rbx We can probably leave out the integration tests as you mentioned they currently only use the display coverage command to test that coverage indicators are shown. The unit tests you added are more then enough for now!