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..2cd056b59aafd0d2ba12ba5cb2121db378647dad 100644
--- a/blimp/BUILD.gn
+++ b/blimp/BUILD.gn
@@ -59,6 +59,14 @@ group("blimp_tests") {
   }
 }
 
+if (is_linux) {
+  test("blimp_browsertests") {
+    deps = [
+      "//blimp/test:engine_browsertests",
+    ]
+  }
+}
+
 test("blimp_unittests") {
   deps = [
     "//blimp/client:app_unit_tests",
@@ -73,10 +81,55 @@ test("blimp_unittests") {
   }
 }
 
-if (is_linux) {
-  test("blimp_browsertests") {
-    deps = [
-      "//blimp/test:engine_browsertests",
-    ]
-  }
+_blimp_tests_runtime_deps = "$root_gen_dir/blimp-tests.runtime_deps"

[maniscalco, 2016/05/27 17:24:40]

I'm thinking this should be inside an is_linux guard since we don't want to
package up tests on any other platforms, right?

This probably applies to all the new build logic added to this file.

Consider moving the is_linux block on line 62 down to the end of the file and
enclosing all the new build logic inside that block just below the
test("blimp_browsertests") block.

Or better yet, could we move all of this into a new .gn/.gni file that's
included here (or included in engine/BUILD.gn?)?

[Jess, 2016/05/27 18:37:54]

Moved into if(is_linux).

Wouldn't we end up with a circular dependency on the blimp_unittests in the new
.gn/.gni file senarios?

+_rebased_blimp_tests_runtime_deps =

[maniscalco, 2016/05/27 17:24:40]

These two new var names feel a bit too generic ("blimp_tests" but it's really
about the engine and the stuff it depends on; no client side).

WDYT about blimp_engine_testing_environment?

blimp_engine_testing_env ?
blimp_engine_validation_env ?

[Jess, 2016/05/27 18:37:54]

Added engine_env in a slightly different manner.  PTAL

Pedantic note: this target also includes the Blimp client tests because we don't
have separate targets for client versus engine (versus common/shared) Blimp test
binaries.  Not difficult to add but would increase the complexity of the build
file without providing significant benefit.

+    rebase_path(_blimp_tests_runtime_deps, root_out_dir)
+
+# Tests for validating potential system environments for running Blimp engine.

[maniscalco, 2016/05/27 17:24:40]

How about something like this:

# This group contains Blimp Engine tests and tests of the various components the
Blimp Engine depends on.  The idea is that if this group of tests pass on a
given system there is a high probability the Engine will also work on that
platform.

(with reasonable line wrapping of course)

[Jess, 2016/05/27 18:37:54]

Done.

+group("blimp_tests_group") {

[maniscalco, 2016/05/27 17:24:40]

blimp_tests_group feels too generic (e.g. we don't ever expect it to contain any
client side stuff).

How about calling it blimp_engine_environment_tests ?

[Jess, 2016/05/27 18:37:54]

Renamed in line with other vars and actions.  Let me know if you prefer
environment to env.

+  testonly = true
+
+  # Additional environment test targets should be added here.
+  # 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 = [
+    ":blimp_unittests",
+    "//base:base_unittests",
+  ]
+
+  write_runtime_deps = _blimp_tests_runtime_deps
+}
+
+_blimp_tests_manifest = "$root_gen_dir/tests-manifest.txt"
+_rebased_blimp_tests_manifest = rebase_path(_blimp_tests_manifest, root_out_dir)
+_rebased_blimp_tests_blacklist =
+    rebase_path("//blimp/tools/tests-manifest-blacklist.txt")
+
+action("generate_blimp_tests_manifest") {
+  testonly = true
+  script = "//blimp/tools/generate-target-manifest.py"
+  args = [
+    "--blacklist",
+    _rebased_blimp_tests_blacklist,
+    "--runtime-deps-file",
+    _rebased_blimp_tests_runtime_deps,
+    "--output",
+    _rebased_blimp_tests_manifest,
+  ]
+  inputs = [
+    _blimp_tests_runtime_deps,
+  ]
+  outputs = [
+    _blimp_tests_manifest,
+  ]
+
+  # By specifying a dependency (not a data_dependency) on :blimp_tests_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_tests_group",
+  ]
 }