-
-
Notifications
You must be signed in to change notification settings - Fork 289
test: Add downstream tests against real external consumers (joern, databricks/dicer) #1868
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
base: master
Are you sure you want to change the base?
Changes from 15 commits
1a0b671
4b04da2
3d57eff
fdec8ec
0c15d4a
2bae3b0
42183a1
edc0295
24daf99
49e34c4
65018e7
6312c23
985353e
205ff3a
8b160a9
bd12fd1
7127013
1983581
c08f1a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| # See README.md in this directory for what these are and how to run one. | ||
| # | ||
| # scala_version below duplicates the pin in the root MODULE.bazel's | ||
|
bor-p-s marked this conversation as resolved.
|
||
| # downstream_consumers.consumer(...) call for the same repo_name, kept in | ||
| # sync by hand -- it can't be load()-ed from the fetched repo instead, since | ||
| # a load() on an external-repo label forces Bazel to fetch that repo just to | ||
| # *load this package*, regardless of the `manual` tag on the targets below. | ||
| load(":downstream_test.bzl", "downstream_test") | ||
|
|
||
| exports_files(["downstream_test_driver.sh", "dicer_protobuf_version.patch"]) | ||
|
|
||
| # joern is Scala-3-only and uses only scala_library/scala_test throughout its | ||
| # tree. | ||
| # | ||
| # The following are excluded because each needs something this environment | ||
| # doesn't have -- none of it is rules_scala's concern: | ||
| # - php2cpg needs a real `php` binary on PATH. | ||
| # - rust2cpg needs `cargo`/`rustc` on PATH. | ||
| # - swiftsrc2cpg times out (its own native toolchain setup, not investigated). | ||
| downstream_test( | ||
| name = "joern_test", | ||
| repo_name = "joern", | ||
| scala_version = "3.7.4", | ||
| targets = [ | ||
| "//...", | ||
| "-//joern-cli/frontends/php2cpg/...", | ||
| "-//joern-cli/frontends/rust2cpg/...", | ||
| "-//joern-cli/frontends/swiftsrc2cpg/...", | ||
| ], | ||
| ) | ||
|
|
||
| # dicer exercises rules_scala's scala_proto/ScalaPB (gRPC) codegen path and | ||
| # pins Scala 2.12 -- both are things joern's Scala-3-only, proto-free tree | ||
|
bor-p-s marked this conversation as resolved.
Outdated
|
||
| # never touches, so this is a second, non-redundant consumer rather than a | ||
| # duplicate of joern_test. | ||
| # | ||
| # Unlike joern above, nothing is excluded here: `caching/util:etcd_suite`, | ||
| # `dicer/common:etcd_bootstrapper_test`, and `dicer/assigner/...` each need | ||
| # a downloaded `etcd_release` binary (`//bazel:etcd.bzl`) hardcoded to a | ||
| # linux-amd64 release, so it should run fine on this repo's own linux-amd64 | ||
| # CI -- it only fails locally, with "etcd exited with code 126", on other | ||
| # architectures (e.g. darwin-arm64). Unconfirmed on real CI, though: this | ||
| # target has no active CI trigger yet (still `manual`, see README). | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 blocked on permissions?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or will the PR need to merged first before it can be first tested?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aww okay, #1870 |
||
| downstream_test( | ||
| name = "dicer_test", | ||
| repo_name = "dicer", | ||
| scala_version = "2.12.21", | ||
| targets = [ | ||
| "//...", | ||
| ], | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| # Downstream tests | ||
|
|
||
| Each fetches a real, independently-maintained external project at a pinned | ||
| commit as a Bazel-native external repo (`MODULE.bazel`'s | ||
| `downstream_consumers` extension, backed by `downstream_repository.bzl`), | ||
| patching in `local_path_override(rules_scala)` so it builds against *this* | ||
| checkout instead of a released version, then runs its own test suite | ||
| against it. | ||
|
|
||
| Every target here is tagged `manual`, so a plain `bazel test //...` (this | ||
| repo's own CI tasks run that wildcard) never picks them up -- cloning and | ||
| testing a full external project is slow and network-dependent. `manual` | ||
| excludes wildcard patterns too (`//test/community_build/...`, | ||
| `//test/community_build:all` -- neither finds any test targets), so name | ||
| each one explicitly instead: | ||
|
|
||
| ```sh | ||
| bazel test --test_env=PATH //test/community_build:joern_test | ||
| ``` | ||
|
|
||
| Run both together the same way, just with both names on one command line: | ||
|
|
||
| ```sh | ||
| bazel test --test_env=PATH //test/community_build:joern_test //test/community_build:dicer_test | ||
| ``` | ||
|
|
||
| (`--test_env=PATH`: the nested `bazel` invocation needs the *consumer's* | ||
| pinned Bazel version, not this checkout's -- see `downstream_test_driver.sh`.) | ||
|
|
||
| Each run executes for real, never from Bazel's test-result cache (the | ||
| targets are tagged `external`): the nested build reads this checkout's live | ||
| source tree, so rules_scala's own sources are code under test without being | ||
| declared inputs of the `sh_test` -- a cached PASS would survive edits to | ||
| them. | ||
|
|
||
| ## What these cover -- and what they deliberately don't | ||
|
|
||
| joern and dicer between them exercise `scala_library`, `scala_test`, | ||
| `scala_binary`, `scala_proto`/ScalaPB, and custom toolchain registration, | ||
| on Scala 3 and 2.12. No consumer here exercises `scala_junit_test`, specs2, | ||
| `scala_library_suite`, twitter_scrooge, scalafmt, semanticdb, or the | ||
|
bor-p-s marked this conversation as resolved.
|
||
| coverage path -- as of mid-2026 no live, publicly buildable Bzlmod project | ||
| using those could be found (the historical heavy users, e.g. Wix's specs2 | ||
| codebase, are closed-source or long unmaintained). Those surfaces are | ||
| covered by this repo's own tests and `examples/` instead; a synthetic | ||
| downstream consumer would only duplicate `examples/` while adding none of | ||
| the realism that is the entire point here. If a real consumer of those APIs | ||
| appears, add it. | ||
|
|
||
| ## Adding a consumer | ||
|
|
||
| 1. Declare it in the root `MODULE.bazel`, via | ||
| `downstream_consumers.consumer(name = ..., remote = ..., commit = ...)`. | ||
| If it needs consumer-specific patching (e.g. dicer's protobuf-version | ||
| override), add a patch file and pass it via that same call's `patches` | ||
| arg (like `http_archive`'s). | ||
| 2. Add a `downstream_test(...)` target in `BUILD`, with its `scala_version` | ||
| as a literal matching the `MODULE.bazel` pin (see the existing targets | ||
| and the comment above them) and a target pattern (prefer `//...` plus | ||
| negative patterns for known-bad exclusions over a hand-picked allowlist). | ||
|
PawelLipski marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # dicer pins protobuf via its own single_version_override, which wins over | ||
| # this checkout's once we're building from inside dicer's checkout | ||
| # (single_version_override only applies from the root module). Drop dicer's | ||
| # pin/patch (for an unrelated legacy Python-toolchain bug) and bump to what | ||
| # this checkout's scala_proto/ScalaPB tooling expects. | ||
| --- a/MODULE.bazel | ||
| +++ b/MODULE.bazel | ||
| @@ -30,9 +30,7 @@ | ||
| bazel_dep(name = "protobuf", version = "30.2") | ||
| single_version_override( | ||
| module_name = "protobuf", | ||
| - patch_strip = 1, | ||
| - patches = ["//bazel:protobuf.patch"], | ||
| - version = "30.2", | ||
| + version = "33.5", | ||
| ) | ||
|
|
||
| # ======================================== |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| """Fetches a real external consumer as a Bazel-native external repo.""" | ||
|
|
||
| def _downstream_consumer_repository_impl(repository_ctx): | ||
| remote = repository_ctx.attr.remote | ||
| commit = repository_ctx.attr.commit | ||
|
|
||
| repository_ctx.execute(["git", "init", "--quiet", "."]) | ||
| repository_ctx.execute(["git", "remote", "add", "origin", remote]) | ||
| result = repository_ctx.execute( | ||
| ["git", "fetch", "--quiet", "--depth", "1", "origin", commit], | ||
| timeout = 20, | ||
| ) | ||
| if result.return_code != 0: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow I'm surprised that Bazel doesn't just fail in such case 😯
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (no action required ofc)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed empirically out of curiosity: wrote a throwaway repository_rule that runs a command with a non-zero exit and skips the check — bazel query completes without complaint. repository_ctx.execute() really does just hand you the exit code and leave fail-fast up to you. |
||
| fail("git fetch failed for %s @ %s:\n%s" % (remote, commit, result.stderr)) | ||
| result = repository_ctx.execute(["git", "checkout", "--quiet", "FETCH_HEAD"]) | ||
| if result.return_code != 0: | ||
| fail("git checkout failed for %s @ %s:\n%s" % (remote, commit, result.stderr)) | ||
|
|
||
| # The whole point: resolve *this* checkout's absolute path via repository_ctx, | ||
| # instead of a caller having to know/compute it in bash. | ||
| rules_scala_dir = str(repository_ctx.path(Label("@rules_scala//:MODULE.bazel")).dirname) | ||
|
|
||
| module_bazel = repository_ctx.path("MODULE.bazel") | ||
| existing = repository_ctx.read(module_bazel) | ||
| repository_ctx.file(module_bazel, existing + ''' | ||
|
|
||
| local_path_override( | ||
| module_name = "rules_scala", | ||
| path = "{rules_scala_dir}", | ||
| ) | ||
| '''.format(rules_scala_dir = rules_scala_dir)) | ||
|
|
||
| for patch in repository_ctx.attr.patches: | ||
| repository_ctx.patch(patch, strip = 1) | ||
|
|
||
| # A downstream_test's `sh_test` needs a real Bazel dependency edge on this | ||
|
bor-p-s marked this conversation as resolved.
|
||
| # repo, so Bazel fetches it before the test runs -- but referencing any | ||
| # label under the consumer's own (real) BUILD files would make Bazel load | ||
| # them, which fails on whatever `bazel_dep`s the consumer declares that | ||
| # aren't in *our* module graph (e.g. joern's `aspect_rules_lint`). This | ||
| # marker package is generated by us, lives alongside the consumer's real | ||
| # files without touching them, and is the only thing `downstream_test` | ||
| # ever references (as a `data` dependency, not a `load()` -- see | ||
| # test/community_build/BUILD for why that distinction matters here). | ||
| repository_ctx.file( | ||
| "_bazel_native_marker/BUILD", | ||
| 'exports_files(["marker.txt"])\n', | ||
| ) | ||
| repository_ctx.file("_bazel_native_marker/marker.txt", commit + "\n") | ||
|
|
||
| _downstream_consumer_repository = repository_rule( | ||
| implementation = _downstream_consumer_repository_impl, | ||
| attrs = { | ||
| "remote": attr.string(mandatory = True), | ||
| "commit": attr.string(mandatory = True), | ||
| "patches": attr.label_list(default = []), | ||
| }, | ||
| ) | ||
|
|
||
| def _downstream_consumers_impl(module_ctx): | ||
| for mod in module_ctx.modules: | ||
| for consumer in mod.tags.consumer: | ||
| _downstream_consumer_repository( | ||
| name = consumer.name, | ||
| remote = consumer.remote, | ||
| commit = consumer.commit, | ||
| patches = consumer.patches, | ||
| ) | ||
|
|
||
| downstream_consumers = module_extension( | ||
| implementation = _downstream_consumers_impl, | ||
| tag_classes = { | ||
| "consumer": tag_class(attrs = { | ||
| "name": attr.string(mandatory = True), | ||
| "remote": attr.string(mandatory = True), | ||
| "commit": attr.string(mandatory = True), | ||
| "patches": attr.label_list(default = []), | ||
| }), | ||
| }, | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| """Bazel-native downstream test: fetches a real external consumer as a | ||
| repository (see downstream_repository.bzl), then in the consumer's own | ||
| checkout, wired against *this* rules_scala checkout: first repins its Maven | ||
| lockfile(s) (`bazel run @maven//:pin`, since the consumer's exact pin may not | ||
| match a version this checkout's third_party repos carry), then runs a | ||
| nested `bazel test` against `targets`. | ||
| """ | ||
|
|
||
| load("@rules_shell//shell:sh_test.bzl", "sh_test") | ||
|
|
||
| def downstream_test( | ||
| name, | ||
| repo_name, | ||
| scala_version, | ||
| targets, | ||
| extra_bazel_flags = "", | ||
| size = "large", | ||
| tags = ["manual", "external", "local", "requires-network"], | ||
| **kwargs): | ||
| """Declares an `sh_test` testing `targets` in the `repo_name` external repo. | ||
|
|
||
| Args: | ||
| name: test target name. | ||
| repo_name: name of the `downstream_consumer_repository` to test. | ||
| scala_version: value to force via --repo_env=SCALA_VERSION (must be | ||
| one this checkout's third_party repos carry). | ||
| targets: list of Bazel target patterns to test in the consumer repo. | ||
| extra_bazel_flags: extra flags forwarded to the nested `bazel test`. | ||
| size: test size; defaults to "large" (nested Bazel invocation, cold | ||
| Maven/git fetch on first run). | ||
| tags: test tags; defaults to | ||
| `["manual", "external", "local", "requires-network"]`. | ||
| `manual` keeps this out of `bazel test //...` (this repo's own CI | ||
| tasks run that wildcard, and cloning + testing a full external | ||
| project on every PR is slow and network-dependent). `external` | ||
| disables test-result caching: the nested build reads this | ||
| checkout's *live* source tree via `local_path_override`, so | ||
| rules_scala's sources are code under test without being declared | ||
| inputs of this `sh_test` -- a cached PASS would survive edits to | ||
| them and go stale (the same unsound-caching trap | ||
| `expect_build_failure_test` fixed by globbing its code under | ||
| test, except here the code under test is the whole repo, out of | ||
| a BUILD-file glob's reach). `local` and `requires-network` match | ||
| `expect_build_failure_test`'s rationale -- the nested build | ||
| fetches external repos and must run outside the sandbox. | ||
| **kwargs: forwarded to the underlying `sh_test`. | ||
| """ | ||
| sh_test( | ||
| name = name, | ||
| srcs = ["//test/community_build:downstream_test_driver.sh"], | ||
| args = [ | ||
| "--marker-rootpath", | ||
| "$(rootpath @{}//_bazel_native_marker:marker.txt)".format(repo_name), | ||
| "--scala-version", | ||
| scala_version, | ||
| "--output-base-name", | ||
| name, | ||
| ] + (["--extra-bazel-flags", extra_bazel_flags] if extra_bazel_flags else []) + [ | ||
| "--", | ||
| ] + targets, | ||
| data = [ | ||
| "@{}//_bazel_native_marker:marker.txt".format(repo_name), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow indeed no other edges to the repo 😯 I would guess some concrete dep would be used instead |
||
| "//test/expect_build_failure:nested_bazel.sh", | ||
| ], | ||
| size = size, | ||
| tags = tags, | ||
| **kwargs | ||
| ) | ||
Uh oh!
There was an error while loading. Please reload this page.