Adding basic unittest manifest generation to blimp/BUILD.gn

Adding basic unittest manifest generation to blimp/BUILD.gn along with appropriate blacklist.

Currently there is no manifest generated for any unittests. Manifests are used to ensure all required files are bundled into a tarball for export and later use with docker.

This change adds targets to create manifests for //blimp:blimp_unittests and //base:base_unittests. The next step would be to use these manifests to bundle the tests into tarballs for export.

BUG=613000
Committed: https://crrev.com/5f84f9b09435c54bc2ee6e52235d7039e6e04295
Cr-Commit-Position: refs/heads/master@{#397471}

[Jess, 2016-05-24 00:07:35]

Description was changed from

==========
Adding basic unittest manifest generation to blimp/BUILD.gn
along with appropriate blacklist.

BUG=613000
==========

to

==========
Adding basic unittest manifest generation to blimp/BUILD.gn along with
appropriate blacklist.

Currently there is no manifest generated for any unittests, preventing bundling
for use with docker.

This change adds targets to create manifests for //blimp:blimp_unittests and
//base:base_unittests. The next step would be to use these manifests to bundle
the tests into docker images for deployment.

BUG=613000
==========

[Jess, 2016-05-24 00:18:29]

jessicag@chromium.org changed reviewers:
+ dpranke@chromium.org

[Jess, 2016-05-24 00:18:30]

PTAL.

https://codereview.chromium.org/1997493002/diff/40001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/40001/blimp/BUILD.gn#newcode102
blimp/BUILD.gn:102: "//base:base_unittests",
dpranke: Please sanity check how I am generating these runtime deps.

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

this more-or-less lgtm to me. Short of actually testing the patch locally
myself, I think it's right.

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn#newcode148
blimp/BUILD.gn:148: }
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.

[maniscalco, 2016-05-24 22:04:47]

maniscalco@chromium.org changed reviewers:
+ maniscalco@chromium.org

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

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn#newcode102
blimp/BUILD.gn:102: group("blimp_env_tests_group") {
Env stands for environment, right?  What do we mean by blimp environment?  I
think I might be missing some context there.

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn#newcode119
blimp/BUILD.gn:119: _rebased_blimp_unittests_blacklist =
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:52:00]

On 2016/05/24 21:29:28, Dirk Pranke wrote:
> this more-or-less lgtm to me. Short of actually testing the patch locally
> myself, I think it's right.
> 
> https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn
> File blimp/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn#newcode148
> blimp/BUILD.gn:148: }
> 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.

Do you have any suggestions on what may be up with analysis failures?
This seems to be something a little bit subtle with dependency chains and
write_runtime_deps I am not understanding.

------
The file:
  //out/Release/gen/blimp-unittests.runtime_deps
is listed as an input or source for the target:
  //blimp:generate_blimp_unittests_manifest
but no targets in the build generate that file.

If you have generated inputs, there needs to be a dependency path between the
two targets in addition to just listing the files. For indirect dependencies,
the intermediate ones must be public_deps. data_deps don't count since they're
only runtime dependencies. If you think a dependency chain exists, it might be
because the chain is private. Try "gn path" to analyze.

[Jess, 2016-05-24 23:53:38]

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn#newcode102
blimp/BUILD.gn:102: group("blimp_env_tests_group") {
On 2016/05/24 22:04:48, maniscalco wrote:
> Env stands for environment, right?  What do we mean by blimp environment?  I
> think I might be missing some context there.

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.

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn#newcode119
blimp/BUILD.gn:119: _rebased_blimp_unittests_blacklist =
On 2016/05/24 22:04:48, maniscalco wrote:
> This may be a silly question, but I want to make sure we think about it:  Do
we
> need/want multiple blacklists?

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.

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn#newcode148
blimp/BUILD.gn:148: }
On 2016/05/24 21:29:28, Dirk Pranke wrote:
> 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.

Done.

[Dirk Pranke, 2016-05-25 01:56:05]

On 2016/05/24 23:52:00, Jess wrote:
> On 2016/05/24 21:29:28, Dirk Pranke wrote:
> > this more-or-less lgtm to me. Short of actually testing the patch locally
> > myself, I think it's right.
> > 
> > https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn
> > File blimp/BUILD.gn (right):
> > 
> >
>
https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn#newcode148
> > blimp/BUILD.gn:148: }
> > 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.
> 
> Do you have any suggestions on what may be up with analysis failures?
> This seems to be something a little bit subtle with dependency chains and
> write_runtime_deps I am not understanding.

Strange; that sure looks like it should be right to me. I'll have to apply the
patch locally and see if I can understand what's going on.

[Dirk Pranke, 2016-05-25 04:50:48]

dpranke@chromium.org changed reviewers:
+ brettw@chromium.org

[Dirk Pranke, 2016-05-25 04:51:48]

On 2016/05/25 01:56:05, Dirk Pranke wrote:
> On 2016/05/24 23:52:00, Jess wrote:
> > Do you have any suggestions on what may be up with analysis failures?
> > This seems to be something a little bit subtle with dependency chains and
> > write_runtime_deps I am not understanding.
> 
> Strange; that sure looks like it should be right to me. I'll have to apply the
> patch locally and see if I can understand what's going on.

+brettw ...

I'm able to get this to pass some times running locally, and fail others. So it
looks
like there's a flaky race (i.e., a bug) somewhere in GN.

@brettw - can you take a look at this?

[maniscalco, 2016-05-25 15:42:58]

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn#newcode102
blimp/BUILD.gn:102: group("blimp_env_tests_group") {
On 2016/05/24 23:53:37, Jess wrote:
> On 2016/05/24 22:04:48, maniscalco wrote:
> > Env stands for environment, right?  What do we mean by blimp environment?  I
> > think I might be missing some context there.
> 
> 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.

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

https://codereview.chromium.org/1997493002/diff/80001/blimp/BUILD.gn#newcode119
blimp/BUILD.gn:119: _rebased_blimp_unittests_blacklist =
On 2016/05/24 23:53:37, Jess wrote:
> On 2016/05/24 22:04:48, maniscalco wrote:
> > This may be a silly question, but I want to make sure we think about it:  Do
> we
> > need/want multiple blacklists?
> 
> 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.

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.

[Jess, 2016-05-25 17:14:42]

jessicag@chromium.org changed reviewers:
+ sriramsr@chromium.org

[Jess, 2016-05-25 22:00:11]

Merged to a single test manifest. PTAL.

[Sriram, 2016-05-26 00:06:20]

https://codereview.chromium.org/1997493002/diff/120001/blimp/tools/tests-mani...
File blimp/tools/tests-manifest-blacklist.txt (right):

https://codereview.chromium.org/1997493002/diff/120001/blimp/tools/tests-mani...
blimp/tools/tests-manifest-blacklist.txt:10: ./libmalloc_wrapper.so
Why are you blacklisting this library? The test will not start without this.

[Jess, 2016-05-26 00:39:39]

https://codereview.chromium.org/1997493002/diff/120001/blimp/tools/tests-mani...
File blimp/tools/tests-manifest-blacklist.txt (right):

https://codereview.chromium.org/1997493002/diff/120001/blimp/tools/tests-mani...
blimp/tools/tests-manifest-blacklist.txt:10: ./libmalloc_wrapper.so
On 2016/05/26 00:06:19, Sriram wrote:
> Why are you blacklisting this library? The test will not start without this.

I was operating under the assumption this shared object was a system file.
Wouldn't we instead want this to come from the system we are running on? Or am I
missing some way in which this library is different than the others listed here?

[Jess, 2016-05-27 16:44:50]

PTAL

https://codereview.chromium.org/1997493002/diff/120001/blimp/tools/tests-mani...
File blimp/tools/tests-manifest-blacklist.txt (right):

https://codereview.chromium.org/1997493002/diff/120001/blimp/tools/tests-mani...
blimp/tools/tests-manifest-blacklist.txt:10: ./libmalloc_wrapper.so
On 2016/05/26 00:39:39, Jess wrote:
> On 2016/05/26 00:06:19, Sriram wrote:
> > Why are you blacklisting this library? The test will not start without this.
> 
> I was operating under the assumption this shared object was a system file.
> Wouldn't we instead want this to come from the system we are running on? Or am
I
> missing some way in which this library is different than the others listed
here?

Confirmed my assumption was wrong on my own. Removed from blacklist.

[maniscalco, 2016-05-27 17:24:41]

Nice, looks much simpler.  Here are my comments...

https://codereview.chromium.org/1997493002/diff/140001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/140001/blimp/BUILD.gn#newcode84
blimp/BUILD.gn:84: _blimp_tests_runtime_deps =
"$root_gen_dir/blimp-tests.runtime_deps"
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?)?

https://codereview.chromium.org/1997493002/diff/140001/blimp/BUILD.gn#newcode85
blimp/BUILD.gn:85: _rebased_blimp_tests_runtime_deps =
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 ?

https://codereview.chromium.org/1997493002/diff/140001/blimp/BUILD.gn#newcode88
blimp/BUILD.gn:88: # Tests for validating potential system environments for
running Blimp engine.
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)

https://codereview.chromium.org/1997493002/diff/140001/blimp/BUILD.gn#newcode89
blimp/BUILD.gn:89: group("blimp_tests_group") {
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]

https://codereview.chromium.org/1997493002/diff/140001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/140001/blimp/BUILD.gn#newcode84
blimp/BUILD.gn:84: _blimp_tests_runtime_deps =
"$root_gen_dir/blimp-tests.runtime_deps"
On 2016/05/27 17:24:40, maniscalco wrote:
> 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?)?

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?

https://codereview.chromium.org/1997493002/diff/140001/blimp/BUILD.gn#newcode85
blimp/BUILD.gn:85: _rebased_blimp_tests_runtime_deps =
On 2016/05/27 17:24:40, maniscalco wrote:
> 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 ?

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.

https://codereview.chromium.org/1997493002/diff/140001/blimp/BUILD.gn#newcode88
blimp/BUILD.gn:88: # Tests for validating potential system environments for
running Blimp engine.
On 2016/05/27 17:24:40, maniscalco wrote:
> 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)

Done.

https://codereview.chromium.org/1997493002/diff/140001/blimp/BUILD.gn#newcode89
blimp/BUILD.gn:89: group("blimp_tests_group") {
On 2016/05/27 17:24:40, maniscalco wrote:
> 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 ?

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

[Jess, 2016-05-27 19:35:49]

jessicag@chromium.org changed reviewers:
+ wez@chromium.org

[Jess, 2016-05-27 19:35:50]

Adding Wez for input on how to handle the testing binary.

In offline discussions with Maniscalco we discussed options for handling the
test binaries.

We think it would be best to provide test binaries for the client and engine
unittests separately. It would remove an unneeded dependency of the engine
environmental tests bundle on client unittests. However we wanted to hear your
thoughts on this change and on if the combined blimp unittest binary should be
kept.

[Jess, 2016-05-27 19:35:53]

Adding Wez for input on how to handle the testing binary.

In offline discussions with Maniscalco we discussed options for handling the
test binaries.

We think it would be best to provide test binaries for the client and engine
unittests separately. It would remove an unneeded dependency of the engine
environmental tests bundle on client unittests. However we wanted to hear your
thoughts on this change and on if the combined blimp unittest binary should be
kept.

[Jess, 2016-06-01 20:55:13]

Isolates have been closed as an alternate option (please contact me off issue if
you disagree or have questions).

PTAL at this change.

[maniscalco, 2016-06-01 22:25:40]

LGTM % a couple nits

https://codereview.chromium.org/1997493002/diff/150001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/150001/blimp/BUILD.gn#newcode108
blimp/BUILD.gn:108: _blimp_engine_env_tests_manifest =
"$root_gen_dir/tests-manifest.txt"
nit: test-manifest.txt is pretty generic, any chance we collide with something
else in chromium?  Maybe prefix with blimp- ?

https://codereview.chromium.org/1997493002/diff/150001/blimp/BUILD.gn#newcode112
blimp/BUILD.gn:112: rebase_path("//blimp/tools/tests-manifest-blacklist.txt")
(same nit as above)

[Jess, 2016-06-01 23:07:24]

https://codereview.chromium.org/1997493002/diff/150001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/150001/blimp/BUILD.gn#newcode108
blimp/BUILD.gn:108: _blimp_engine_env_tests_manifest =
"$root_gen_dir/tests-manifest.txt"
On 2016/06/01 22:25:40, maniscalco wrote:
> nit: test-manifest.txt is pretty generic, any chance we collide with something
> else in chromium?  Maybe prefix with blimp- ?

Done.

https://codereview.chromium.org/1997493002/diff/150001/blimp/BUILD.gn#newcode112
blimp/BUILD.gn:112: rebase_path("//blimp/tools/tests-manifest-blacklist.txt")
On 2016/06/01 22:25:40, maniscalco wrote:
> (same nit as above)

I lean towards keeping the name here for two reasons:
1. Since this is a checked-in file with a full path rather than a generated file
there should not be concerns about naming collisions.
2. This follows the convention set by the existing blacklist
(//blimp/tools/engine-manifest-blacklist.txt).

[Sriram, 2016-06-01 23:16:46]

lgtm

[Wez, 2016-06-02 00:01:42]

nit: Re CL description it's not immediately clear what the manifest is for - are
we talking about a manifest describing what to bundle into a Docker bundle, for
example?

[Wez, 2016-06-02 00:07:20]

lgtm

https://codereview.chromium.org/1997493002/diff/170001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/170001/blimp/BUILD.gn#newcode99
blimp/BUILD.gn:99: # everything it requires) is a build-time dependency only.
IIUC you're saying that write_runtime_deps will only list out the data-deps,
since they express run-time dependencies, while normal deps are assumed to be
only build-time, so get omitted?

https://codereview.chromium.org/1997493002/diff/170001/blimp/BUILD.gn#newcode114
blimp/BUILD.gn:114: action("generate_blimp_engine_env_tests_manifest") {
Is there something elsewhere that will depend on this, or is it intended to be
built explicitly

https://codereview.chromium.org/1997493002/diff/170001/blimp/BUILD.gn#newcode132
blimp/BUILD.gn:132: # By specifying a dependency (not a data_dependency) on
:blimp_tests_group,
nit: Why does it need to be a public_dep?

[Jess, 2016-06-02 00:08:10]

Description was changed from

==========
Adding basic unittest manifest generation to blimp/BUILD.gn along with
appropriate blacklist.

Currently there is no manifest generated for any unittests, preventing bundling
for use with docker.

This change adds targets to create manifests for //blimp:blimp_unittests and
//base:base_unittests. The next step would be to use these manifests to bundle
the tests into docker images for deployment.

BUG=613000
==========

to

==========
Adding basic unittest manifest generation to blimp/BUILD.gn along with
appropriate blacklist.

Currently there is no manifest generated for any unittests. Manifests are used
to ensure all required files are bundled into a tarball for export and later use
with docker.

This change adds targets to create manifests for //blimp:blimp_unittests and
//base:base_unittests. The next step would be to use these manifests to bundle
the tests into tarballs for export.

BUG=613000
==========

[Jess, 2016-06-02 00:24:28]

PTAL

https://codereview.chromium.org/1997493002/diff/170001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/170001/blimp/BUILD.gn#newcode99
blimp/BUILD.gn:99: # everything it requires) is a build-time dependency only.
On 2016/06/02 00:07:20, Wez wrote:
> IIUC you're saying that write_runtime_deps will only list out the data-deps,
> since they express run-time dependencies, while normal deps are assumed to be
> only build-time, so get omitted?

Yes.

https://codereview.chromium.org/1997493002/diff/170001/blimp/BUILD.gn#newcode114
blimp/BUILD.gn:114: action("generate_blimp_engine_env_tests_manifest") {
On 2016/06/02 00:07:20, Wez wrote:
> Is there something elsewhere that will depend on this, or is it intended to be
> built explicitly

The action that creates the tarball will depend on this file.  I have a separate
dependent change queued up for it.

https://codereview.chromium.org/1997493002/diff/170001/blimp/BUILD.gn#newcode132
blimp/BUILD.gn:132: # By specifying a dependency (not a data_dependency) on
:blimp_tests_group,
On 2016/06/02 00:07:20, Wez wrote:
> nit: Why does it need to be a public_dep?

It doesn't. Fixed.

[Jess, 2016-06-02 01:00:28]

The CQ bit was checked by jessicag@chromium.org

[Jess, 2016-06-02 01:00:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, maniscalco@chromium.org, sriramsr@chromium.org,
wez@chromium.org
Link to the patchset: https://codereview.chromium.org/1997493002/#ps190001
(title: "Nit fix.")

[commit-bot: I haz the power, 2016-06-02 01:03:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1997493002/190001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1997493002/190001

[commit-bot: I haz the power, 2016-06-02 02:04:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-02 02:04:18]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[maniscalco, 2016-06-02 14:41:54]

https://codereview.chromium.org/1997493002/diff/150001/blimp/BUILD.gn
File blimp/BUILD.gn (right):

https://codereview.chromium.org/1997493002/diff/150001/blimp/BUILD.gn#newcode112
blimp/BUILD.gn:112: rebase_path("//blimp/tools/tests-manifest-blacklist.txt")
On 2016/06/01 23:07:24, Jess wrote:
> On 2016/06/01 22:25:40, maniscalco wrote:
> > (same nit as above)
> 
> I lean towards keeping the name here for two reasons:
> 1. Since this is a checked-in file with a full path rather than a generated
file
> there should not be concerns about naming collisions.
> 2. This follows the convention set by the existing blacklist
> (//blimp/tools/engine-manifest-blacklist.txt).

Oops, my mistake.  I missed that this one is in //blimp and not $root_gen_dir. 
Keeping it as is SGTM.

[Jess, 2016-06-02 16:53:35]

The CQ bit was checked by jessicag@chromium.org

[commit-bot: I haz the power, 2016-06-02 16:54:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1997493002/190001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1997493002/190001

[commit-bot: I haz the power, 2016-06-02 18:17:37]

Description was changed from

==========
Adding basic unittest manifest generation to blimp/BUILD.gn along with
appropriate blacklist.

Currently there is no manifest generated for any unittests. Manifests are used
to ensure all required files are bundled into a tarball for export and later use
with docker.

This change adds targets to create manifests for //blimp:blimp_unittests and
//base:base_unittests. The next step would be to use these manifests to bundle
the tests into tarballs for export.

BUG=613000
==========

to

==========
Adding basic unittest manifest generation to blimp/BUILD.gn along with
appropriate blacklist.

Currently there is no manifest generated for any unittests. Manifests are used
to ensure all required files are bundled into a tarball for export and later use
with docker.

This change adds targets to create manifests for //blimp:blimp_unittests and
//base:base_unittests. The next step would be to use these manifests to bundle
the tests into tarballs for export.

BUG=613000
==========

[commit-bot: I haz the power, 2016-06-02 18:17:39]

Committed patchset #11 (id:190001)

[commit-bot: I haz the power, 2016-06-02 18:21:16]

Description was changed from

==========
Adding basic unittest manifest generation to blimp/BUILD.gn along with
appropriate blacklist.

Currently there is no manifest generated for any unittests. Manifests are used
to ensure all required files are bundled into a tarball for export and later use
with docker.

This change adds targets to create manifests for //blimp:blimp_unittests and
//base:base_unittests. The next step would be to use these manifests to bundle
the tests into tarballs for export.

BUG=613000
==========

to

==========
Adding basic unittest manifest generation to blimp/BUILD.gn along with
appropriate blacklist.

Currently there is no manifest generated for any unittests. Manifests are used
to ensure all required files are bundled into a tarball for export and later use
with docker.

This change adds targets to create manifests for //blimp:blimp_unittests and
//base:base_unittests. The next step would be to use these manifests to bundle
the tests into tarballs for export.

BUG=613000

Committed: https://crrev.com/5f84f9b09435c54bc2ee6e52235d7039e6e04295
Cr-Commit-Position: refs/heads/master@{#397471}
==========

[commit-bot: I haz the power, 2016-06-02 18:21:17]

Patchset 11 (id:??) landed as
https://crrev.com/5f84f9b09435c54bc2ee6e52235d7039e6e04295
Cr-Commit-Position: refs/heads/master@{#397471}