Add target base_unittests_run and tools/isolate/isolate.py.

Add target base_unittests_run and tools/isolate/isolate.py.

This target uses isolate.py to run the unit test in a separate directory.

BUG=98636
TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=125386

[M-A Ruel, 2012-02-28 18:50:35]

Still playing with the idea. I'm currently fixing it on all OSes.

1. I'm also probably going to not redirect stdout since it's less useful.
Instead, options.results would just be touched if the test passes.

2. I'm probably going to do use a DEPS dependency:
- devs get the default dependency, which is this code.
- builders get the dependency that generates hash table, using a custom_deps.
- testers wouldn't fetch the sources anyway.

Any opinion?

[M-A Ruel, 2012-02-28 21:02:28]

On 2012/02/28 18:50:35, Marc-Antoine Ruel wrote:
> Still playing with the idea. I'm currently fixing it on all OSes.
> 
> 1. I'm also probably going to not redirect stdout since it's less useful.
> Instead, options.results would just be touched if the test passes.

Done.

I fixed directory handling and added some more dependencies. In theory directory
handling is bad but for now it helps me make progress.

MessagePumpLibeventTest.DeleteWatcher consistenly crash here, I'm not sure why
yet.

I'm about to test on other platforms.

> 2. I'm probably going to do use a DEPS dependency:
> - devs get the default dependency, which is this code.
> - builders get the dependency that generates hash table, using a custom_deps.
> - testers wouldn't fetch the sources anyway.

So the idea is that I won't check in tools/isolate. Instead it will be a
dependency. Also, I'd probably set all buildbot masters to use a dummy
tools/isolate in the meantime since we don't want all the 'compile' steps to run
all tests (!)

This means I need to restart all the masters to add the custom_deps. :(

> Any opinion?

[M-A Ruel, 2012-03-02 02:17:16]

Now in functional state. I'm not 100% happy with the "tests_run" variable name,
I'm open to suggestions.

willchan for base/.
szager or rogerta for the rest, whoever feels like reviewing first.

[willchan no longer on Chromium, 2012-03-02 02:31:08]

302 Found
Server: willchan
Location: mark@chromium.org

It looks like the only change is a gyp change, and I don't know what an isolate
is, so mark@ is probably more appropriate. And I hate code reviews :P

[M-A Ruel, 2012-03-02 16:18:07]

See background information at
http://dev.chromium.org/developers/testing/isolated-testing

[Roger Tawa OOO till Jul 10th, 2012-03-02 16:47:49]

Looks fine.  A few nits below.

Can you add style-guide type comments where missing?
Can you add some unit tests?

https://chromiumcodereview.appspot.com/9513003/diff/19001/tools/isolate/tree_...
File tools/isolate/tree_creator.py (right):

https://chromiumcodereview.appspot.com/9513003/diff/19001/tools/isolate/tree_...
tools/isolate/tree_creator.py:106: blacklist: Files to inconditionally ignore.
inconditionally -->unconditionally

https://chromiumcodereview.appspot.com/9513003/diff/19001/tools/isolate/tree_...
tools/isolate/tree_creator.py:110: outdir, indir, infiles, action, blacklist))
is the indenting OK here?

https://chromiumcodereview.appspot.com/9513003/diff/19001/tools/isolate/tree_...
tools/isolate/tree_creator.py:166: relpath = os.path.join(root, dirpath)
is this relative or absolute?

[M-A Ruel, 2012-03-02 21:28:30]

On 2012/03/02 16:47:49, Roger Tawa wrote:
> Can you add style-guide type comments where missing?

I just added one line comments. This is following PEP8 and friends albeit with
+2 indentation.

> Can you add some unit tests?

Added a basic integration test.

https://chromiumcodereview.appspot.com/9513003/diff/19001/tools/isolate/tree_...
File tools/isolate/tree_creator.py (right):

https://chromiumcodereview.appspot.com/9513003/diff/19001/tools/isolate/tree_...
tools/isolate/tree_creator.py:106: blacklist: Files to inconditionally ignore.
On 2012/03/02 16:47:49, Roger Tawa wrote:
> inconditionally -->unconditionally

done

https://chromiumcodereview.appspot.com/9513003/diff/19001/tools/isolate/tree_...
tools/isolate/tree_creator.py:110: outdir, indir, infiles, action, blacklist))
On 2012/03/02 16:47:49, Roger Tawa wrote:
> is the indenting OK here?

That's the indenting I generally use, +4 for function, +2 for structures.

https://chromiumcodereview.appspot.com/9513003/diff/19001/tools/isolate/tree_...
tools/isolate/tree_creator.py:166: relpath = os.path.join(root, dirpath)
On 2012/03/02 16:47:49, Roger Tawa wrote:
> is this relative or absolute?

This was indeed wrong, fixed.

[Roger Tawa OOO till Jul 10th, 2012-03-02 21:35:52]

lgtm

Thanks MA!

[M-A Ruel, 2012-03-06 02:15:45]

On 2012/03/02 02:31:08, willchan wrote:
> 302 Found
> Server: willchan
> Location: mailto:mark@chromium.org
> 
> It looks like the only change is a gyp change, and I don't know what an
isolate
> is, so mark@ is probably more appropriate. And I hate code reviews :P

Mark, can you review on Tuesday? Otherwise, I'll TBR the patch.

[Mark Mentovai, 2012-03-06 22:05:02]

https://chromiumcodereview.appspot.com/9513003/diff/22002/base/base.gyp
File base/base.gyp (right):

https://chromiumcodereview.appspot.com/9513003/diff/22002/base/base.gyp#newco...
base/base.gyp:453: 'data/file_util_unittest/',
Specifying a directory as an input is undefined. What are you trying to achieve
with this?

[M-A Ruel, 2012-03-06 22:19:51]

https://chromiumcodereview.appspot.com/9513003/diff/22002/base/base.gyp
File base/base.gyp (right):

https://chromiumcodereview.appspot.com/9513003/diff/22002/base/base.gyp#newco...
base/base.gyp:453: 'data/file_util_unittest/',
On 2012/03/06 22:05:02, Mark Mentovai wrote:
> Specifying a directory as an input is undefined. What are you trying to
achieve
> with this?

For now, I don't list each files in the test data directories because I think
it'll be overwhelming. So I made the tool to be able to handle directories even
if the build tool can't detect any modification. I may revisit this decision
later if we want a more correct solution with regards to data files. For very
large number of data files, it will probably make sense to put them in a
separate .gypi. I plan to base my decision based on how well it goes in
practices as I make progress on the following tests.

For example, base_unittests.exe on windows currently depends on
third_party/python_26. It'd be quite overwhelming to ask the build too to stat()
each of the files in this directory and its subdirectories.

[Mark Mentovai, 2012-03-06 22:23:08]

You can make the tool accept a directory name on the command line, then. Just
don’t declare it to GYP as an explicit 'input' to an action. That will result in
undefined behavior including, possibly, your action not running at all.

[M-A Ruel, 2012-03-07 00:05:35]

On 2012/03/06 22:23:08, Mark Mentovai wrote:
> You can make the tool accept a directory name on the command line, then. Just
> don’t declare it to GYP as an explicit 'input' to an action. That will result
in
> undefined behavior including, possibly, your action not running at all.

You are right, fixed.

[Mark Mentovai, 2012-03-07 00:06:42]

LGTM. I only reviewed your .gyp/.gypi files.

[commit-bot: I haz the power, 2012-03-07 00:49:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/9513003/33001

[commit-bot: I haz the power, 2012-03-07 04:24:53]

Try job failure for 9513003-33001 (retry) on win_rel for steps "unit_tests,
browser_tests, ui_tests".
It's a second try, previously, steps "unit_tests, browser_tests, ui_tests"
failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-03-07 13:08:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/9513003/33001

[commit-bot: I haz the power, 2012-03-07 14:39:58]

Change committed as 125386