Allow bazel test //... to run out-of-the-box #4150
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes a few of the TODOs in the WORKSPACE file and changes a lot about the popular_repos tests. This commit also provides a default value of `--java_runtime_version=remotejdk_21` just for the bazel_test in lcov_coverage_test.go so that contributors who don't have a JAVA_HOME on the host system can still get the lcov_coverage_test.go to pass by default. Oh, and it also fixes a TODO in the repository's `.bazelrc` file.
As a contributor to the project, I expect to be able to clone the repository, go to the master branch and run `bazel test //...` and have it work. But, before this change, just like the CI system, contributors were also expected to run `./tests/core/cgo/generate_imported_dylib.sh` manually. Well ... no more!
That script should no longer be run.
The Ubuntu build is breaking because jdk21 is not supported on that OS, and the Windows build is failing because some of the tools pprof and diffp are not available on Windows.
fmeum
approved these changes
Oct 21, 2024
|
Amazing PR. Thanks so much 🙏 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
This PR fixes 3 issues which caused an invocation of
bazel test //...in a clean checkout of the repository to faildepending on potential contributor's development environment.
generate_imported_dylib.shgo env GOTOOLCHAINsetFor 1, this PR wraps the
generate_imported_dylib.shin genrules sothat the build will just automatically create the dynamic libraries
appropriate for the platform.
For 2, the PR adds a
--java_runtime_version=remotejdk_21argument tothe one test in
lcov_coverage_test.gothat needs to be able to builda Java rule.
For 3, the PR upgrades
gazellleto version 0.39.1 and updates theexcludes lists in the
popular_repos.pyfile to handle the latestversions of those popular repos. This upgrade addresses the problem
because bazel-contrib/bazel-gazelle#1858 is
fixed in that version of gazelle.
BONUS: Also, there were a few TODOs left in the
WORKSPACEand.bazelrcfiles which were addressed by upgrading to the latestgazelleversion. So, this PR cleans those up at the same time.Which issues(s) does this PR fix?
Fixes #4149
Other notes for review
I have tested this on the following two machines:
macos (M3 arm64)
linux (amd64)
In both cases, I can clone the repository into an fresh directory hop
onto the
build-fixesbranch and run:And, it actually has all 452 tests passing within 30 minutes or so.
On both machines,
go env GOTOOLCHAINisgo1.23.2and$JAVA_HOMEis not set on either machine.