Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(coverage): Basic test coverage support #366

Merged

Conversation

vogelsgesang
Copy link
Collaborator

@vogelsgesang vogelsgesang commented Apr 5, 2024

This commit lays the foundation for displaying coverage results from bazel coverage.

Currently, the functionality is only exposed through the user-defined tasks in the tasks.json. It is thereby a bit hard to discover. But this is fine for the time being, because coverage still has a couple of rough edges anyway. As soon as it is more stable, we should add builtin commands and expose coverage runs also in the "Bazel Build Target" tree.

Changes in this commit:

  • Bumps the VS Code version to 1.88, i.e. the first VS Code version which supports the test coverage API.
  • Upgrades to ES2022. I wanted to use replaceAll which was introduced in ES2021. VS Code 1.88 is based on Node 18 which in turn is based on V8 10.1. V8 10.18 supports ECMA-262 also known as ES2023. However, ES2023 is not yet available a target language in the tsconfig.json. Furthermore, Firefox does not fully support ES2023, yet. While web browsers are currently not relevant, they might become so in the future if we want to turn this into a browser-enabled VSCode extension. An upgrade to ES2021 would have been sufficient, but I went directly to ES2022 because it might some of the other new features might also turn out useful.
  • Introduces a custom LCOV parser. I could not find any other high-quality open-source parser. E.g., most other parser don't properly parse function names with : and / or , in them.
  • Introduces test cases for that custom LCOV parser.
  • Add the test cases to GitHub Actions. I followed the instructions from https://code.visualstudio.com/api/working-with-extensions/continuous-integration.

Future work:

  • Support for branch coverage
  • Demangling of function names
  • Builtin commands to trigger coverage runs & offer them in the "Bazel Build Tree"

Tested with: Java, C++, Go, Rust
Untested: Python, Swift, Kotlin, Scala and many more

This is the first step towards #362

This commit lays the foundation for displaying coverage results
from `bazel coverage`.

For the time being, the functionality is only exposed through the
user-defined tasks in the `tasks.json`. It is thereby a bit hard to
discover. But this is fine for the time being, because coverage still
has a couple of rough edges anyway. As soon as it is more stable, we
should add builtin commands and expose coverage runs also in the "Bazel
Build Target" tree.

Changes in this commit:
* Bumps the VS Code version to 1.88, i.e. the first VS Code version
  which supports the test coverage API.
* Upgrades to ES2022.  I wanted to use `replaceAll` which was introduced
  in ES2021.
  VS Code 1.88 is based on Node 18 which in turn is based on V8 10.1. V8
  10.18 supports ECMA-262 also known as ES2023.  However, ES2023 is not
  yet available a target language in the `tsconfig.json`.  Furthermore,
  Firefox does not fully support ES2023, yet. While web browsers are
  currently not relevant, they might become so in the future if we want
  to turn this into a browser-enabled VSCode extension.
  An upgrade to ES2021 would have been sufficient, but I went directly
  to ES2022 because it might some of the other new features might also
  turn out useful.
* Introduces a custom LCOV parser. I could not find any other
  high-quality open-source parser. E.g., most other parser don't
  properly parse function names with `:` and / or `,` in them.
* Introduces test cases for that custom LCOV parser.

Future work:
* Support for branch coverage
* Demangling of function names
* Builtin commands to trigger coverage runs & offer them in the "Bazel
  Build Tree"

Tested with: Java, C++, Go, Rust
Untested: Python, Swift, Kotlin, Scala and many more

This is the first step towards bazel-contrib#362
@vogelsgesang
Copy link
Collaborator Author

See microsoft/vscode#209697 for a couple of screenshots

@vogelsgesang vogelsgesang changed the title feat: Basic test coverage support feat(coverage): Basic test coverage support Apr 6, 2024
@vogelsgesang vogelsgesang mentioned this pull request Apr 8, 2024
@cameron-martin
Copy link
Collaborator

I've reviewed everything but the lcov parser, and it looks good. I've run out of time this evening, so I'll have to review the lcov parser another day.

@vogelsgesang
Copy link
Collaborator Author

I am still interested in merging this, but I will wait with resolving the merge conflicts until after the review 🙂

Copy link
Collaborator

@cameron-martin cameron-martin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, looks good.

@vogelsgesang
Copy link
Collaborator Author

No worries, I know this was a larger, more complicated commit. Thanks for taking the time to review this in-depth instead of just give me a "probably good-enough approval"! Good reviews are important to the project in the long run 🙂

@vogelsgesang vogelsgesang merged commit 0d293b8 into bazel-contrib:master May 2, 2024
2 checks passed
vogelsgesang added a commit to vogelsgesang/vscode-bazel that referenced this pull request May 2, 2024
During the rebase of bazel-contrib#366, I accidentally duplicated the `test` key
inside the `scripts` in `package.json`. There was no warning whatsoever
and CI was still green, but didn't actually run the `vscode-test` tests.

This commit fixes the mishap
cameron-martin added a commit that referenced this pull request May 2, 2024
During the rebase of #366, I accidentally duplicated the `test` key
inside the `scripts` in `package.json`. There was no warning whatsoever
and CI was still green, but didn't actually run the `vscode-test` tests.

This commit fixes the mishap

Co-authored-by: Cameron Martin <cameronmartin123@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants