Adding basic unittest manifest generation to blimp/BUILD.gn

blimp/BUILD.gn

Index: blimp/BUILD.gn
diff --git a/blimp/BUILD.gn b/blimp/BUILD.gn
index f7191103828b6b8f10dbcb5e758415c398555c9e..a5730d8d3e24af1468dfad3f3dfdd951e653771a 100644
--- a/blimp/BUILD.gn
+++ b/blimp/BUILD.gn
@@ -59,6 +59,18 @@ group("blimp_tests") {
   }
 }
 
+if (is_linux) {
+  test("blimp_browsertests") {
+    deps = [
+      "//blimp/test:engine_browsertests",
+    ]
+  }
+}
+
+_blimp_unittests_runtime_deps = "$root_gen_dir/blimp-unittests.runtime_deps"
+_rebased_blimp_unittests_runtime_deps =
+    rebase_path(_blimp_unittests_runtime_deps, root_out_dir)
+
 test("blimp_unittests") {
   deps = [
     "//blimp/client:app_unit_tests",
@@ -71,12 +83,100 @@ test("blimp_unittests") {
   if (is_linux) {
     deps += [ "//blimp/engine:unit_tests" ]
   }
+
+  write_runtime_deps = _blimp_unittests_runtime_deps
 }
 
-if (is_linux) {
-  test("blimp_browsertests") {
-    deps = [
-      "//blimp/test:engine_browsertests",
-    ]
-  }
+group("blimp_unittests_group") {
+  testonly = true
+
+  public_deps = [
+    ":blimp_unittests",
+  ]
+}
+
+_blimp_env_tests_runtime_deps = "$root_gen_dir/blimp-env-tests.runtime_deps"
+_rebased_blimp_env_tests_runtime_deps =
+    rebase_path(_blimp_env_tests_runtime_deps, root_out_dir)
+
+group("blimp_env_tests_group") {

[maniscalco, 2016/05/24 22:04:48]

Env stands for environment, right?  What do we mean by blimp environment?  I
think I might be missing some context there.

[Jess, 2016/05/24 23:53:37]

Added more comments to this group and on what should be included in the
data_deps here.  We can chat more offline about what context should be included
here.

[maniscalco, 2016/05/25 15:42:57]

Yeah, let's chat OOB.  I'm still not quite sure I understand how to think about
env.  I can't tell if it's

1. blimp test environment - an environment with all the stuff necessary to run
blimp tests and tests of the things blimp depends on.

2. blimp runtime environment for the engine

3. blimp runtime environment for engine and engine tests

+  testonly = true
+
+  # Executable targets and those executable targets' transitive
+  # dependencies are not considered unless that executable is listed in
+  # "data_deps". Otherwise, GN assumes that the executable (and
+  # everything it requires) is a build-time dependency only.
+  data_deps = [
+    "//base:base_unittests",
+  ]
+
+  write_runtime_deps = _blimp_env_tests_runtime_deps
+}
+
+_blimp_unittests_manifest = "$root_gen_dir/unittests-manifest.txt"
+_rebased_blimp_unittests_manifest =
+    rebase_path(_blimp_unittests_manifest, root_out_dir)
+_rebased_blimp_unittests_blacklist =

[maniscalco, 2016/05/24 22:04:48]

This may be a silly question, but I want to make sure we think about it:  Do we
need/want multiple blacklists?

[Jess, 2016/05/24 23:53:37]

Not silly at all. The existing blacklists are somewhat overlapping and, at the
moment, combining them would not result in any of the manifests missing needed
files.

In theory there could be a desire to blacklist a file from one manifest but
include it in another, but that seems remote now. OTOH, I'd expect any
additional tests to be added to the existing blimp_env_test_group so I would not
expect more blacklists to maintain.

At the moment it could go either way depending on how often we expect the
blacklist(s) to change and if we they they would ever be incompatible.

[maniscalco, 2016/05/25 15:42:57]

OK, let's chat about this one OOB as well.  The motivation I had in mind for
keeping on blacklist is that it would be easier to maintain.  So far, keeping
our engine blacklist accurate and up to date has been more effort than I would
have expected.  I'm assuming that going forward when we need to update one, we
probably need to update all.

+    rebase_path("//blimp/tools/unittests-manifest-blacklist.txt")
+
+action("generate_blimp_unittests_manifest") {
+  testonly = true
+  script = "//blimp/tools/generate-target-manifest.py"
+  args = [
+    "--blacklist",
+    _rebased_blimp_unittests_blacklist,
+    "--runtime-deps-file",
+    _rebased_blimp_unittests_runtime_deps,
+    "--output",
+    _rebased_blimp_unittests_manifest,
+  ]
+  inputs = [
+    _blimp_unittests_runtime_deps,
+  ]
+  outputs = [
+    _blimp_unittests_manifest,
+  ]
+
+  # By specifying a dependency (not a data_dependency) on
+  # :blimp_unittests_group,
+  # we can be sure that everything is built before the action is
+  # complete (though not necessarily before we generate the manifest
+  # itself).
+  public_deps = [
+    ":blimp_unittests_group",
+  ]
+}

[Dirk Pranke, 2016/05/24 21:29:28]

nit: I would probably move lines 116-148 above line 98, to keep the unittest
stuff together and the env stuff togerther, rather than keeping the manifests
together.

[Jess, 2016/05/24 23:53:37]

Done.

+
+_blimp_env_tests_manifest = "$root_gen_dir/env-tests-manifest.txt"
+_rebased_blimp_env_tests_manifest =
+    rebase_path(_blimp_env_tests_manifest, root_out_dir)
+_rebased_blimp_env_tests_blacklist =
+    rebase_path("//blimp/tools/env-tests-manifest-blacklist.txt")
+
+action("generate_blimp_env_tests_manifest") {
+  testonly = true
+  script = "//blimp/tools/generate-target-manifest.py"
+  args = [
+    "--blacklist",
+    _rebased_blimp_env_tests_blacklist,
+    "--runtime-deps-file",
+    _rebased_blimp_env_tests_runtime_deps,
+    "--output",
+    _rebased_blimp_env_tests_manifest,
+  ]
+  inputs = [
+    _blimp_env_tests_runtime_deps,
+  ]
+  outputs = [
+    _blimp_env_tests_manifest,
+  ]
+
+  # By specifying a dependency (not a data_dependency) on
+  # :blimp_env_tests_group,
+  # we can be sure that everything is built before the action is
+  # complete (though not necessarily before we generate the manifest
+  # itself).
+  deps = [
+    ":blimp_env_tests_group",
+  ]
 }