Skip to content

Commit

Permalink
Migrate tests to use Gazelle's integration test runner
Browse files Browse the repository at this point in the history
Advantages:
* allows us to delete a lot of fragile code re-implementing Gazelle's
  internal workfows.
* Each top level testdata directory is a standalone test target which
  can run in parallel, and is independently cached.
* Stdout, stderr, and exit code are assertable artifacts of these tests.
  This is particularly good because right now we have stderr we probably
  don't expect, and this makes that hard to miss. (We can fix up
  removing that stderr in the future).
* Performs a full resolve, rather than a partial one (which means we
  actually see the generated BUILD files as a user would). Several of
  the changes in this PR are adding dependencies as a result of this.
* No longer uses a fake resolver. We can fake things out with
  `# gazelle:resolve directives` or with an actual maven_install.json if
  we wish.
* Allows easily passing flags to gazelle.

Drawbacks:
* We can no longer assert on the output of generate, only resolve. I
  think this is probably fine - if we want to assert on detected
  imports, that should be probably be done at a Java unit test level
  anyway.
* Currently each of the test targets takes 30 seconds to complete. This
  is because we wait for the Java server to terminate before the test is
  considered complete, and we don't proactively kill it.
  bazel-contrib#70 and
  bazel-contrib/bazel-gazelle#1325 will address
  this.

Open questions:
* Which tests do we want to have a maven_install.json for vs use
  `# gazelle:resolve` for vs just assert there's a stderr log?
* What's the future of `resolve_test`? I don't think it's super valuable
  as-is, but for now I've just preserved it (and moved some helpers into
  it) - let's have this discussion post-merge.

How to review this change:
* All of the BUILD.want.gen files are deleted.
* BUILD.want.res has been renamed BUILD.out - any content changes here
  are because our test pipeline is more faithful to reality than the old
  one was.
* Look at each of the `expectedStderr.txt` files and if anything looks
  terribly offensive, we should fix it.
  • Loading branch information
illicitonion committed Aug 29, 2022
1 parent 49dff43 commit 1590026
Show file tree
Hide file tree
Showing 115 changed files with 1,665 additions and 1,056 deletions.
9 changes: 0 additions & 9 deletions java/gazelle/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

# gazelle:exclude testdata
# gazelle:exclude def.bzl

go_library(
Expand Down Expand Up @@ -35,29 +34,21 @@ go_library(
],
)

# Needs to be no-remote as the test invokes "bazel info" (see java/private/bazel/bazel.go)
# In the production use this recursive call to "bazel info" derives the directory where the external files are
# loaded, allowing for the dependency verification.
go_test(
name = "gazelle_test",
size = "medium",
srcs = [
"generate_integration_test.go",
"generate_test.go",
"integration_base_test.go",
"resolve_integration_test.go",
"resolve_test.go",
],
data = glob(["testdata/**"]),
embed = [":gazelle"],
tags = ["no-remote"],
deps = [
"//java/gazelle/private/sorted_set",
"@bazel_gazelle//config:go_default_library",
"@bazel_gazelle//label:go_default_library",
"@bazel_gazelle//language:go_default_library",
"@bazel_gazelle//language/proto:go_default_library",
"@bazel_gazelle//merger:go_default_library",
"@bazel_gazelle//pathtools:go_default_library",
"@bazel_gazelle//repo:go_default_library",
"@bazel_gazelle//resolve:go_default_library",
Expand Down
78 changes: 0 additions & 78 deletions java/gazelle/generate_integration_test.go

This file was deleted.

Loading

0 comments on commit 1590026

Please sign in to comment.