In archive_layout_test_results.py, simplify argument handling.

In archive_layout_test_results.py, simplify argument handling.

As far as I know, archive_layout_test_results.py is currently
always called from the chromium_tests recipe module, after running
layout tests, for the "archive_webkit_tests_results" step. In
practice, this is always called like this:

archive_webkit_tests_results
python -u .../archive_layout_test_results.py \
  --results-dir /b/rr/tmpa5gR1b/w/layout-test-results \
  --build-dir /b/c/b/mac_layout/src/out \
  --build-number 25264 \
  --builder-name 'WebKit Mac10.10' \
  --gs-bucket gs://chromium-layout-test-archives \
  --staging-dir /b/c/chrome_staging

That is, results-dir is always given and is an absolute path,
and build number and builder name are always given.

In this CL:
 - use argparse rather than optparse
 - remove --build-properties and --factory-properties args
 - make build number and builder name required
 - make results dir required, and don't treat it as a
   relative path relative to the build dir (since in practice
   it isn't).

BUG=655810
Committed: https://chromium.googlesource.com/chromium/tools/build/+/ece187bdc4c389344de942753c06aaa69d0e880e

[qyearsley, 2016-10-17 18:26:12]

Description was changed from

==========
In archive_layout_test_results.py, simplify argument handling.

As far as I know, archive_layout_test_results.py is currently
always called from the chromium_tests recipe module, after running
layout tests, for the "archive_webkit_tests_results" step. In
practice, this is always called like this:

archive_webkit_tests_results
python -u .../archive_layout_test_results.py \
  --results-dir /b/rr/tmpa5gR1b/w/layout-test-results \
  --build-dir /b/c/b/mac_layout/src/out \
  --build-number 25264 \
  --builder-name 'WebKit Mac10.10' \
  --gs-bucket gs://chromium-layout-test-archives \
  --staging-dir /b/c/chrome_staging

That is, results-dir is always given and is an absolute path,
and build number and builder name are always given.

With this in mind, these arguments can be made required so
the script can be simplified slightly.

BUG=655810
==========

to

==========
In archive_layout_test_results.py, simplify argument handling.

As far as I know, archive_layout_test_results.py is currently
always called from the chromium_tests recipe module, after running
layout tests, for the "archive_webkit_tests_results" step. In
practice, this is always called like this:

archive_webkit_tests_results
python -u .../archive_layout_test_results.py \
  --results-dir /b/rr/tmpa5gR1b/w/layout-test-results \
  --build-dir /b/c/b/mac_layout/src/out \
  --build-number 25264 \
  --builder-name 'WebKit Mac10.10' \
  --gs-bucket gs://chromium-layout-test-archives \
  --staging-dir /b/c/chrome_staging

That is, results-dir is always given and is an absolute path,
and build number and builder name are always given.

In this CL:
 - use argparse rather than optparse
 - remove --build-properties and --factory-properties args
 - make build number and builder name required
 - make results dir required, and don't treat it as a
   relative path relative to the build dir (since in practice
   it isn't).

BUG=655810
==========

[qyearsley, 2016-10-17 18:26:13]

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

[qyearsley, 2016-10-18 16:25:33]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 16:25:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 16:35:14]

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

[commit-bot: I haz the power, 2016-10-18 16:35:15]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2016-10-18 20:03:03]

lgtm

https://codereview.chromium.org/2429583002/diff/40001/scripts/slave/chromium/...
File scripts/slave/chromium/archive_layout_test_results.py (right):

https://codereview.chromium.org/2429583002/diff/40001/scripts/slave/chromium/...
scripts/slave/chromium/archive_layout_test_results.py:135:
parser.add_argument('--results-dir', required=True,
Nice, didn't know about the required flag.

[qyearsley, 2016-10-18 20:32:27]

On 2016/10/18 at 20:03:03, dpranke wrote:
> lgtm
> 
>
https://codereview.chromium.org/2429583002/diff/40001/scripts/slave/chromium/...
> File scripts/slave/chromium/archive_layout_test_results.py (right):
> 
>
https://codereview.chromium.org/2429583002/diff/40001/scripts/slave/chromium/...
> scripts/slave/chromium/archive_layout_test_results.py:135:
parser.add_argument('--results-dir', required=True,
> Nice, didn't know about the required flag.

Yep :-)

This is something in argparse but not optparse. optparse has the following note
in its documentation:

  "note that the phrase “required option” is self-contradictory in English.
  optparse doesn’t prevent you from implementing required options, but
  doesn’t give you much help at it either."
    - https://docs.python.org/2/library/optparse.html#terminology

And argparse has this note:

 "Note: Required options are generally considered bad form because users expect
 options to be optional, and thus they should be avoided when possible."
   - https://docs.python.org/3/library/argparse.html#required

I guess the idea is that required args should be positional args, and if you
have more
than 1 or 2 positional args, then maybe you want to use some kind of config
file...
but in this case, the command doesn't have to be super user-friendly, since it's
usually not invoked directly by humans.

[qyearsley, 2016-10-18 20:32:31]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2016-10-18 20:32:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 20:42:52]

Description was changed from

==========
In archive_layout_test_results.py, simplify argument handling.

As far as I know, archive_layout_test_results.py is currently
always called from the chromium_tests recipe module, after running
layout tests, for the "archive_webkit_tests_results" step. In
practice, this is always called like this:

archive_webkit_tests_results
python -u .../archive_layout_test_results.py \
  --results-dir /b/rr/tmpa5gR1b/w/layout-test-results \
  --build-dir /b/c/b/mac_layout/src/out \
  --build-number 25264 \
  --builder-name 'WebKit Mac10.10' \
  --gs-bucket gs://chromium-layout-test-archives \
  --staging-dir /b/c/chrome_staging

That is, results-dir is always given and is an absolute path,
and build number and builder name are always given.

In this CL:
 - use argparse rather than optparse
 - remove --build-properties and --factory-properties args
 - make build number and builder name required
 - make results dir required, and don't treat it as a
   relative path relative to the build dir (since in practice
   it isn't).

BUG=655810
==========

to

==========
In archive_layout_test_results.py, simplify argument handling.

As far as I know, archive_layout_test_results.py is currently
always called from the chromium_tests recipe module, after running
layout tests, for the "archive_webkit_tests_results" step. In
practice, this is always called like this:

archive_webkit_tests_results
python -u .../archive_layout_test_results.py \
  --results-dir /b/rr/tmpa5gR1b/w/layout-test-results \
  --build-dir /b/c/b/mac_layout/src/out \
  --build-number 25264 \
  --builder-name 'WebKit Mac10.10' \
  --gs-bucket gs://chromium-layout-test-archives \
  --staging-dir /b/c/chrome_staging

That is, results-dir is always given and is an absolute path,
and build number and builder name are always given.

In this CL:
 - use argparse rather than optparse
 - remove --build-properties and --factory-properties args
 - make build number and builder name required
 - make results dir required, and don't treat it as a
   relative path relative to the build dir (since in practice
   it isn't).

BUG=655810

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/ece187bdc4c389344de9...
==========

[commit-bot: I haz the power, 2016-10-18 20:42:52]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/tools/build/+/ece187bdc4c389344de9...