[Android] Enable uploading instrumentation tests to flakiness dashboard.

[Android] Enable uploading instrumentation tests to flakiness dashboard.

This is the client-side change for uploading the following
instrumentation test types:

androidwebview_instrumentation_tests
chromiumtestshell_instrumentation_tests
contentshell_instrumentation_tests

Actuall uploading of results will first be done
on a staging server.

BUG=150801
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=176758

[mkosiba (inactive), 2013-01-14 12:07:55]

LGTM

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/test_resu...
File build/android/pylib/test_result.py (right):

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/test_resu...
build/android/pylib/test_result.py:178: raise Exception('Invalid test type.')
maybe just log && return?

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/test_resu...
build/android/pylib/test_result.py:182: # Use http://test-results.appspot.com
once we're confidant this works.
nit: confident

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/utils/fla...
File build/android/pylib/utils/flakiness_dashboard_results_uploader.py (right):

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/utils/fla...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:76: return
_is_git_directory(parent)
Won't this cause us to recurse until we hit '/' ? Maybe explicitly stop at
CHROME_SRC?

[craigdh, 2013-01-14 17:43:41]

lgtm.

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/utils/fla...
File build/android/pylib/utils/flakiness_dashboard_results_uploader.py (right):

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/utils/fla...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:121: upstream
= tests_type != 'Chromium_Android_Instrumentation'
nit: parentheses would be nice here.

[frankf, 2013-01-14 18:54:41]

@chase, can you take a look at use of slave utils in this patch?

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/test_resu...
File build/android/pylib/test_result.py (right):

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/test_resu...
build/android/pylib/test_result.py:178: raise Exception('Invalid test type.')
On 2013/01/14 12:07:55, Martin Kosiba wrote:
> maybe just log && return?

Done.

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/test_resu...
build/android/pylib/test_result.py:182: # Use http://test-results.appspot.com
once we're confidant this works.
Indeed. Done.
On 2013/01/14 12:07:55, Martin Kosiba wrote:
> nit: confident

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/utils/fla...
File build/android/pylib/utils/flakiness_dashboard_results_uploader.py (right):

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/utils/fla...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:76: return
_is_git_directory(parent)
On 2013/01/14 12:07:55, Martin Kosiba wrote:
> Won't this cause us to recurse until we hit '/' ? Maybe explicitly stop at
> CHROME_SRC?

Done.

https://codereview.chromium.org/11885010/diff/1/build/android/pylib/utils/fla...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:121: upstream
= tests_type != 'Chromium_Android_Instrumentation'
On 2013/01/14 17:43:41, craigdh wrote:
> nit: parentheses would be nice here.

Done.

[cmp, 2013-01-14 18:58:38]

Isaac, can you take a look at Frank's CL?

[Isaac (away), 2013-01-14 19:59:44]

I have a couple comments but in general this looks really good!

https://chromiumcodereview.appspot.com/11885010/diff/9001/build/android/pylib...
File build/android/pylib/test_result.py (right):

https://chromiumcodereview.appspot.com/11885010/diff/9001/build/android/pylib...
build/android/pylib/test_result.py:177: if test_type != 'Instrumentation':
Might be nice to also upload unit tests but agree instrumentation tests are far
more likely to be flaky.

https://chromiumcodereview.appspot.com/11885010/diff/9001/build/android/pylib...
build/android/pylib/test_result.py:186: 'ChromiumTestShell',
Any reason this one is referring to main apk instead of its test apk?  Does it
matter?

https://chromiumcodereview.appspot.com/11885010/diff/9001/build/android/pylib...
File build/android/pylib/utils/flakiness_dashboard_results_uploader.py (right):

https://chromiumcodereview.appspot.com/11885010/diff/9001/build/android/pylib...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:14:
sys.path.append(os.path.join(os.path.dirname(os.path.realpath( __file__ )),
Nit: put abspath outside of os.path.join so it resolves the '..'

 realpath probably not necessary since it resolves symlinks.

https://chromiumcodereview.appspot.com/11885010/diff/9001/build/android/pylib...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:78: return
_is_git_directory(parent)
We could alternatively look for the absence of a .svn folder, which is in every
directory and wouldn't require a parent traversal.

https://chromiumcodereview.appspot.com/11885010/diff/9001/build/android/pylib...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:86:
command_line = ['git', 'log', '-1', '--pretty=oneline']
nit you can have git output just the sha with the right argument to --pretty

https://chromiumcodereview.appspot.com/11885010/diff/9001/build/android/pylib...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:124: if
upstream:
Would be a safer to pull this from the factory_properties json that we already
have, see bb_device_steps.py for an example of decoding for optionparser. 
Directory structure is also not reliable for builder lookup.

[frankf, 2013-01-14 22:54:42]

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/test_r...
File build/android/pylib/test_result.py (right):

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/test_r...
build/android/pylib/test_result.py:177: if test_type != 'Instrumentation':
Yep, that's the next step.

On 2013/01/14 19:59:44, Isaac wrote:
> Might be nice to also upload unit tests but agree instrumentation tests are
far
> more likely to be flaky.

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/test_r...
build/android/pylib/test_result.py:186: 'ChromiumTestShell',
Good catch.

On 2013/01/14 19:59:44, Isaac wrote:
> Any reason this one is referring to main apk instead of its test apk?  Does it
> matter?

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
File build/android/pylib/utils/flakiness_dashboard_results_uploader.py (right):

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:14:
sys.path.append(os.path.join(os.path.dirname(os.path.realpath( __file__ )),
On 2013/01/14 19:59:44, Isaac wrote:
> Nit: put abspath outside of os.path.join so it resolves the '..'
> 
>  realpath probably not necessary since it resolves symlinks. 

Done.

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:78: return
_is_git_directory(parent)
This gives us a strong guarantee of this being a git directory. The traversal is
negligible. 

On 2013/01/14 19:59:44, Isaac wrote:
> We could alternatively look for the absence of a .svn folder, which is in
every
> directory and wouldn't require a parent traversal.

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:86:
command_line = ['git', 'log', '-1', '--pretty=oneline']
On 2013/01/14 19:59:44, Isaac wrote:
> nit you can have git output just the sha with the right argument to --pretty

Done.

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:124: if
upstream:
Currently, the mastername is specified as:
"mastername": "chromium.fyi"

Whereas, I need the actual name "ChormiumFYI". I added a TODO here, since this
shouldn't block the CL and we can revisit this.

On 2013/01/14 19:59:44, Isaac wrote:
> Would be a safer to pull this from the factory_properties json that we already
> have, see bb_device_steps.py for an example of decoding for optionparser. 
> Directory structure is also not reliable for builder lookup.

[Isaac (use chromium), 2013-01-14 23:37:27]

lgtm

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
File build/android/pylib/utils/flakiness_dashboard_results_uploader.py (right):

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:78: return
_is_git_directory(parent)
On 2013/01/14 22:54:42, frankf wrote:
> This gives us a strong guarantee of this being a git directory. The traversal
is
> negligible. 
> 
> On 2013/01/14 19:59:44, Isaac wrote:
> > We could alternatively look for the absence of a .svn folder, which is in
> every
> > directory and wouldn't require a parent traversal. 

What about the situation where you have an svn checkout inside a gitignored
folder of a git repo?  This case could occur on buildbot slaves:

/b/build: git repo
/b/build/slave/<name>/build/src/ <chromium svn repo>

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:124: if
upstream:
On 2013/01/14 22:54:42, frankf wrote:
> Currently, the mastername is specified as:
> "mastername": "chromium.fyi"
> 
> Whereas, I need the actual name "ChormiumFYI". I added a TODO here, since this
> shouldn't block the CL and we can revisit this.

Sounds good to me, but as this increases the risk of a build repo change
breaking the bot, I would like to see this fixed before this code runs on the
main waterfall.

[frankf, 2013-01-14 23:58:47]

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
File build/android/pylib/utils/flakiness_dashboard_results_uploader.py (right):

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:78: return
_is_git_directory(parent)
In that case, we'll encounter .svn first, and don't bother with this function.

On 2013/01/14 23:37:27, Isaac (use chromium acct) wrote:
> On 2013/01/14 22:54:42, frankf wrote:
> > This gives us a strong guarantee of this being a git directory. The
traversal
> is
> > negligible. 
> > 
> > On 2013/01/14 19:59:44, Isaac wrote:
> > > We could alternatively look for the absence of a .svn folder, which is in
> > every
> > > directory and wouldn't require a parent traversal. 
> 
> What about the situation where you have an svn checkout inside a gitignored
> folder of a git repo?  This case could occur on buildbot slaves:
> 
> /b/build: git repo
> /b/build/slave/<name>/build/src/ <chromium svn repo>

[Isaac (use chromium), 2013-01-15 00:03:07]

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
File build/android/pylib/utils/flakiness_dashboard_results_uploader.py (right):

https://codereview.chromium.org/11885010/diff/9001/build/android/pylib/utils/...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:78: return
_is_git_directory(parent)
On 2013/01/14 23:58:47, frankf wrote:
> In that case, we'll encounter .svn first, and don't bother with this function.

SGTM

https://codereview.chromium.org/11885010/diff/8003/build/android/pylib/utils/...
File build/android/pylib/utils/flakiness_dashboard_results_uploader.py (right):

https://codereview.chromium.org/11885010/diff/8003/build/android/pylib/utils/...
build/android/pylib/utils/flakiness_dashboard_results_uploader.py:92: return
output[0:40]
nit: output.strip()?