Add src-side launcher for checkdeps

Add src-side launcher for checkdeps

This is first step towards establishing an API between src and recipes.
The goal is to convert all Test subclasses from recipes' steps.py
into scripts like this one.

BUG=422235
R=iannucci@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/875bab4d269a04c1cac398d8e49f28417f9052b3

[Paweł Hajdan Jr., 2014-10-09 14:39:20]

phajdan.jr@chromium.org changed reviewers:
+ iannucci@chromium.org

[Paweł Hajdan Jr., 2014-10-09 14:39:20]

Hey Robbie, I started implementing what you mentioned to me during my last MTV
visit. Please let me know what you think.

If this looks good, I'll file a bug and follow up with a recipe-side change, and
then continue converting other Test subclasses.

[iannucci, 2014-10-10 08:53:50]

LGTM, though I would like to see a 1-page doc (or even .5 pages) with the
proposed api. I understand that it would change over time, but just to get it
written down to look at can help (and other people can look too).

https://codereview.chromium.org/645593002/diff/1/testing/scripts/checkdeps.py
File testing/scripts/checkdeps.py (right):

https://codereview.chromium.org/645593002/diff/1/testing/scripts/checkdeps.py...
testing/scripts/checkdeps.py:36: with open(tempfile_path) as f:
does this have to run on windows? if not, you can use
tempfile.NamedTemporaryFile which should get cleaned up for you.

In any event, I would move the removal of tempfile_path much closer to the
things that are using it.

We've also used:

@contextmanager
def temporary_file():
  fd, tempfile_path = tempfile.mkstemp()
  os.close(fd)
  try:
    yield tempfile_path
  finally:
    os.remove(tempfile_path)

# and in your function
with tempoarary_file() as fname:
  run_command(..., fname)
  with open(fname) as f:
    ... load(f)

Just to keep the logic for the tempfile stuff in one function instead of spread
out.

https://codereview.chromium.org/645593002/diff/1/testing/scripts/checkdeps.py...
testing/scripts/checkdeps.py:56: parser = argparse.ArgumentParser()
why not subparsers? e.g. `cmd run --output /path/to/json`

https://codereview.chromium.org/645593002/diff/1/testing/scripts/checkdeps.py...
testing/scripts/checkdeps.py:62: if args.mode == 'run':
then argparse does this for you

https://codereview.chromium.org/645593002/diff/1/testing/scripts/checkdeps.py...
testing/scripts/checkdeps.py:65: print 'Unhandled mode %r' % args.mode
you should use parser.error for this sort of thing. exits 1, prints help, etc.

[Paweł Hajdan Jr., 2014-10-10 09:21:20]

https://codereview.chromium.org/645593002/diff/1/testing/scripts/checkdeps.py
File testing/scripts/checkdeps.py (right):

https://codereview.chromium.org/645593002/diff/1/testing/scripts/checkdeps.py...
testing/scripts/checkdeps.py:36: with open(tempfile_path) as f:
On 2014/10/10 08:53:50, iannucci wrote:
> does this have to run on windows? if not, you can use
> tempfile.NamedTemporaryFile which should get cleaned up for you.
> 
> In any event, I would move the removal of tempfile_path much closer to the
> things that are using it.
> 
> We've also used:
> 
> @contextmanager
> def temporary_file():
>   fd, tempfile_path = tempfile.mkstemp()
>   os.close(fd)
>   try:
>     yield tempfile_path
>   finally:
>     os.remove(tempfile_path)
> 
> # and in your function
> with tempoarary_file() as fname:
>   run_command(..., fname)
>   with open(fname) as f:
>     ... load(f)
> 
> Just to keep the logic for the tempfile stuff in one function instead of
spread
> out.

Good suggestion, thanks for it and applied. Yes, this needs to run on Windows so
I can't keep the file opened by this script.

https://codereview.chromium.org/645593002/diff/1/testing/scripts/checkdeps.py...
testing/scripts/checkdeps.py:56: parser = argparse.ArgumentParser()
On 2014/10/10 08:53:49, iannucci wrote:
> why not subparsers? e.g. `cmd run --output /path/to/json`

Done.

[iannucci, 2014-10-10 09:30:30]

lgtm

https://codereview.chromium.org/645593002/diff/40001/testing/scripts/checkdep...
File testing/scripts/checkdeps.py (right):

https://codereview.chromium.org/645593002/diff/40001/testing/scripts/checkdep...
testing/scripts/checkdeps.py:47: result_set = set()
I would de-indent here. No reason to have the file still around while you do the
rest of this stuff

https://codereview.chromium.org/645593002/diff/40001/testing/scripts/checkdep...
testing/scripts/checkdeps.py:68: run_parser.set_defaults(func=mode_run)
fancy! I didn't know that trick, but it makes sense to me :)

[Paweł Hajdan Jr., 2014-10-10 10:50:43]

https://codereview.chromium.org/645593002/diff/40001/testing/scripts/checkdep...
File testing/scripts/checkdeps.py (right):

https://codereview.chromium.org/645593002/diff/40001/testing/scripts/checkdep...
testing/scripts/checkdeps.py:47: result_set = set()
On 2014/10/10 09:30:30, iannucci wrote:
> I would de-indent here. No reason to have the file still around while you do
the
> rest of this stuff

Done.

[commit-bot: I haz the power, 2014-10-10 10:52:51]

Patchset 3 (id:??) landed as
https://crrev.com/875bab4d269a04c1cac398d8e49f28417f9052b3
Cr-Commit-Position: refs/heads/master@{#299086}

[Paweł Hajdan Jr., 2014-10-10 10:52:54]

Committed patchset #3 (id:80001) manually as
875bab4d269a04c1cac398d8e49f28417f9052b3 (presubmit successful).