Tinder / bazel-diff

Performs Bazel Target Diffing between two revisions in Git, allowing for Test Target Selection and Selective Building
Other
397 stars 59 forks source link

Rework in Kotlin #123

Closed Malinskiy closed 2 years ago

Malinskiy commented 2 years ago

Here is a non-exhaustive list of stuff done in the branch:

  1. Rewrite in Kotlin
  2. New heuristic for picking up source targets during hash calculation even when the target was not returned by bazel query
  3. Moved integration test into the main codebase and just in general more tests. New coverage is 84%
  4. Further parallelisation of source with hashing + target hashing (~20% faster for a large monorepo). Concurrency races are now solved
  5. Reworked commands since get-impacted-targets doesn’t even need bazel in the first place, so less arguments. This changes the way you access the CLI though
Malinskiy commented 2 years ago

I tried to setup the codecoverage for the project, but JRE8 is incompatible with the coverage code in the bazel apparently, so this in theory would only work for JRE11. Problem is that the output of code coverage $(bazel info output_path)/_coverage/_coverage_report.dat is always empty on the Github Runner and I can't reproduce this locally (where it passes without issues). Will try to tackle it next week

Malinskiy commented 2 years ago

Turns out that using latest version of bazel, i.e. Using unreleased version at commit d46269fe2397b4612a654543ba6fed45c2cdbdea doesn't produce the coverage data. I've replaced the ci config for now with the latest stable @tinder-maxwellelliott . What do you want to do? We can leave it at stable or wait until it's fixed upstream. Don't know if bazel is aware of this though, haven't checked.

As for the coverage - I tried using the commenting action, but it doesn't work with forked repositories since GITHUB_TOKEN has read-only access to the main repo from an action running for a fork. A better way may be to use something like coveralls, I believe they support lcov format but I'm not sure if this would make any difference to the permissions problem

Malinskiy commented 2 years ago

Cleaned up the history a bit: there was a lot of trial and error involved in making this work

tinder-maxwellelliott commented 2 years ago

I tried to setup the codecoverage for the project, but JRE8 is incompatible with the coverage code in the bazel apparently, so this in theory would only work for JRE11. Problem is that the output of code coverage $(bazel info output_path)/_coverage/_coverage_report.dat is always empty on the Github Runner and I can't reproduce this locally (where it passes without issues). Will try to tackle it next week

Can we create a separate workflow for JRE11 and then get coverage data on that at least? I think its good to have a test run on idk8, but we don't need coverage from both

Malinskiy commented 2 years ago

@tinder-maxwellelliott that is exactly what’s done in this PR https://github.com/Tinder/bazel-diff/actions/runs/2337802484 coverage archive is available only from the jre11

tinder-maxwellelliott commented 2 years ago

@tinder-maxwellelliott that is exactly what’s done in this PR https://github.com/Tinder/bazel-diff/actions/runs/2337802484 coverage archive is available only from the jre11

🤦🏻 saw that as I was reviewing

Malinskiy commented 2 years ago

We should also update the readme with the new command bazel-diff generate-hashes. Both have less options, e.g. get-impacted-targets doesn't need bazel info anymore since it's not even used for the diff logic

tinder-maxwellelliott commented 2 years ago

Will do, testing in our repo now and will update accordingly