[WebView] Fix the run_tests.sh script and add documentation.

[WebView] Fix the run_tests.sh script and add documentation.

Fix the run_tests.sh script to use the new wrapper and add
documentation.

BUG=686114
Review-Url: https://codereview.chromium.org/2659843003
Cr-Commit-Position: refs/heads/master@{#450062}
Committed: https://chromium.googlesource.com/chromium/src/+/2441837767de19419b168ee54b8234598db57f69

[timvolodine, 2017-01-27 18:23:16]

timvolodine@chromium.org changed reviewers:
+ torne@chromium.org

[Torne, 2017-01-30 14:27:22]

https://codereview.chromium.org/2659843003/diff/1/android_webview/tools/syste...
File android_webview/tools/system_webview_shell/test/data/run_tests.sh (right):

https://codereview.chromium.org/2659843003/diff/1/android_webview/tools/syste...
android_webview/tools/system_webview_shell/test/data/run_tests.sh:14: #    and
make sure it is enabled.
Isn't the intention of the autogenerated scripts to handle these kinds of steps
for us?

https://codereview.chromium.org/2659843003/diff/1/android_webview/tools/syste...
android_webview/tools/system_webview_shell/test/data/run_tests.sh:25:
SCRIPT_BUILD_DIR="../../../../../out/Debug/"
I don't think it's reasonable to hardcode this output directory - now that we
have gn most people don't use out/Debug (at least I don't) and there's no
particular required directory location.

Shouldn't we instead be making the generated script do all the right stuff to
run the tests, and getting rid of this script? Or am I not understanding the
test setup?

[timvolodine, 2017-02-04 02:42:22]

https://codereview.chromium.org/2659843003/diff/1/android_webview/tools/syste...
File android_webview/tools/system_webview_shell/test/data/run_tests.sh (right):

https://codereview.chromium.org/2659843003/diff/1/android_webview/tools/syste...
android_webview/tools/system_webview_shell/test/data/run_tests.sh:14: #    and
make sure it is enabled.
On 2017/01/30 14:27:22, Torne wrote:
> Isn't the intention of the autogenerated scripts to handle these kinds of
steps
> for us?

right, this is a bit elaborate I'll double-check what the wrapper script does
exactly. However some initial install has to happen anyway for it to work on a
new device.

https://codereview.chromium.org/2659843003/diff/1/android_webview/tools/syste...
android_webview/tools/system_webview_shell/test/data/run_tests.sh:25:
SCRIPT_BUILD_DIR="../../../../../out/Debug/"
On 2017/01/30 14:27:22, Torne wrote:
> I don't think it's reasonable to hardcode this output directory - now that we
> have gn most people don't use out/Debug (at least I don't) and there's no
> particular required directory location.

good point, so maybe just have a parameter (with out/Debug as default)

> 
> Shouldn't we instead be making the generated script do all the right stuff to
> run the tests, and getting rid of this script? Or am I not understanding the
> test setup?

so, this script has the ability to do some rebaselining for the 'layout' tests
expectations, which is kind of 'special' in this case. As I understand it the
generated scripts are purely for running the tests similar to what
test_runner.py used to do..

[timvolodine, 2017-02-04 02:43:09]

+cc:jbudorick@ FYI

[timvolodine, 2017-02-10 18:35:59]

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

[commit-bot: I haz the power, 2017-02-10 18:36:32]

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

[timvolodine, 2017-02-10 18:51:28]

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

[commit-bot: I haz the power, 2017-02-10 18:51:57]

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

[timvolodine, 2017-02-10 19:00:52]

alright, thanks for the comments!

I have updated the documentation (basically we only need to have a proper
webview installed + build the test target). Also renamed the script to
run_tests_rebaseline because that's what it does and added the option to provide
a custom build directory.

PTAL

[commit-bot: I haz the power, 2017-02-10 19:04:38]

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

[commit-bot: I haz the power, 2017-02-10 19:04:40]

Dry run: This issue passed the CQ dry run.

[Torne, 2017-02-13 11:17:40]

Oh, we only need to use this script to rebaseline?

I guess the scripts can't easily install the actual WebView for you because
there's to many possible configurations for this. We need to document that
process somewhere, but it should probably be in the actual build instructions
(in the docs folder), not inside a script that people mostly don't need to
actually run.

[Torne, 2017-02-13 11:17:50]

LGTM

[timvolodine, 2017-02-13 18:47:55]

On 2017/02/13 11:17:40, Torne wrote:
> Oh, we only need to use this script to rebaseline?

yes, originally it was for running tests as well, but now we have a generated
script for that

> 
> I guess the scripts can't easily install the actual WebView for you because
> there's to many possible configurations for this. We need to document that
> process somewhere, but it should probably be in the actual build instructions
> (in the docs folder), not inside a script that people mostly don't need to
> actually run.

ack )

[timvolodine, 2017-02-13 18:48:01]

The CQ bit was checked by timvolodine@chromium.org

[timvolodine, 2017-02-13 18:48:18]

The CQ bit was unchecked by timvolodine@chromium.org

[timvolodine, 2017-02-13 18:54:52]

The CQ bit was checked by timvolodine@chromium.org

[timvolodine, 2017-02-13 18:54:52]

The patchset sent to the CQ was uploaded after l-g-t-m from torne@chromium.org
Link to the patchset: https://codereview.chromium.org/2659843003/#ps60001
(title: "update documentation and copyright")

[commit-bot: I haz the power, 2017-02-13 18:55:41]

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, 2017-02-13 18:59:31]

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

[commit-bot: I haz the power, 2017-02-13 18:59:32]

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

[timvolodine, 2017-02-13 20:16:04]

The CQ bit was checked by timvolodine@chromium.org

[commit-bot: I haz the power, 2017-02-13 20:16:42]

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, 2017-02-13 20:28:30]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487016964404360,
"parent_rev": "3764134e53831644d51c780f3d9b3a50f8d43865", "commit_rev":
"2441837767de19419b168ee54b8234598db57f69"}

[commit-bot: I haz the power, 2017-02-13 20:29:12]

Description was changed from

==========
[WebView] Fix the run_tests.sh script and add documentation.

Fix the run_tests.sh script to use the new wrapper and add
documentation.

BUG=686114
==========

to

==========
[WebView] Fix the run_tests.sh script and add documentation.

Fix the run_tests.sh script to use the new wrapper and add
documentation.

BUG=686114

Review-Url: https://codereview.chromium.org/2659843003
Cr-Commit-Position: refs/heads/master@{#450062}
Committed:
https://chromium.googlesource.com/chromium/src/+/2441837767de19419b168ee54b82...
==========

[commit-bot: I haz the power, 2017-02-13 20:29:15]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/2441837767de19419b168ee54b82...