Archiving of AppRTC APK in client.webrtc waterfall

Archiving of AppRTC APK in client.webrtc waterfall

To ease manual testing, the AppRTCDemo artifacts from the Android builders are being archived to Google storage for the following builders:
  Android32 Builder
  Android32 Builder (dbg)
  Android64 Builder
  Android64 Builder (dbg)

BUG=647242
Committed: https://chromium.googlesource.com/chromium/tools/build/+/a2b056bd69e365ff4c2a9bfa961ad8e13565470f

[ehmaldonado_chromium, 2016-09-16 13:51:19]

ehmaldonado@chromium.org changed reviewers:
+ kjellander@chromium.org

[kjellander_chromium, 2016-09-18 17:20:39]

Description was changed from

==========
Archiving of AppRTC APK in client.webrtc waterfall

To easy manual testing, we'd like to archive the AppRTCDemo artifacts
from the Android builders to Google storage for the following builders:
  Android32 Builder
  Android32 Builder (dbg)
  Android64 Builder
  Android64 Builder (dbg)

BUG=647242
==========

to

==========
Archiving of AppRTC APK in client.webrtc waterfall

To ease manual testing, the AppRTCDemo artifacts from the Android builders are
being archived to Google storage for the following builders:
  Android32 Builder
  Android32 Builder (dbg)
  Android64 Builder
  Android64 Builder (dbg)

BUG=647242
==========

[kjellander_chromium, 2016-09-18 17:33:24]

This will work, but I'd prefer to move some things out from the recipe into the
recipe module instead.

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
File
scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_android32_builder.json
(right):

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_android32_builder.json:303:
"gs://chromium-webrtc/Android32_Builder/AppRTCDemo_apk_170242.zip"
With the suggested approach above, this will be
gs://chromium-webrtc/client.webrtc/Android32 Builder
(dbg)/AppRTCDemo_apk_170242.zip

Which makes more sense, since the master-name directories already exists in the
bucket (and we want to ensure we don't have collisions if we accidentally use
the same buildernames for builders in multiple waterfalls).

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
File scripts/slave/recipes/webrtc/standalone.py (right):

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
scripts/slave/recipes/webrtc/standalone.py:50: if api.properties['buildername']
in ('Android32 Builder',
I'd like to keep builder-details like this closer to the actual builder config
in
https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/webrtc/bu...

Can you make a new custom property like 'archive_apprtc': True?

Like this:
'Android32 Builder': {
        'recipe_config': 'webrtc_android',
        'chromium_config_kwargs': {
          'BUILD_CONFIG': 'Release',
          'TARGET_PLATFORM': 'android',
          'TARGET_ARCH': 'arm',
          'TARGET_BITS': 32,
        },
        'bot_type': 'builder',
        'testing': {'platform': 'linux'},
        'triggers': [
          'Android32 Tests (L Nexus5)',
          'Android32 Tests (L Nexus7.2)',
        ],
        'archive_apprtc': True,
      },

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
scripts/slave/recipes/webrtc/standalone.py:63:
api.commit_position.parse_revision(webrtc.revision_cp))
Instead of this, you can use self.revision_cp if you use the approach described
in the next comment (and you'll also be able to remove the commit_position
dependency of the recipe again).

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
scripts/slave/recipes/webrtc/standalone.py:64: api.gsutil.upload(zip_path,
'chromium-webrtc', destination,
Then we could re-use the existing package_build method and put this step in
there instead:
https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/webrtc/ap...

Like:
if self.bot_config.get('archive_apprtc', False):
  # Do the steps above, but re-use the upload_url variable that already points
to the <GS bucket>/client.webrtc/Android32 Builder (dbg) directory.

That way, the recipe stays much more readable.

[ehmaldonado_chromium, 2016-09-19 06:50:37]

PTAL

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
File
scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_android32_builder.json
(right):

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_android32_builder.json:303:
"gs://chromium-webrtc/Android32_Builder/AppRTCDemo_apk_170242.zip"
On 2016/09/18 17:33:23, kjellander_chromium wrote:
> With the suggested approach above, this will be
> gs://chromium-webrtc/client.webrtc/Android32 Builder
> (dbg)/AppRTCDemo_apk_170242.zip
> 
> Which makes more sense, since the master-name directories already exists in
the
> bucket (and we want to ensure we don't have collisions if we accidentally use
> the same buildernames for builders in multiple waterfalls).

Done.

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
File scripts/slave/recipes/webrtc/standalone.py (right):

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
scripts/slave/recipes/webrtc/standalone.py:50: if api.properties['buildername']
in ('Android32 Builder',
On 2016/09/18 17:33:23, kjellander_chromium wrote:
> I'd like to keep builder-details like this closer to the actual builder config
> in
>
https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/webrtc/bu...
> 
> Can you make a new custom property like 'archive_apprtc': True?
> 
> Like this:
> 'Android32 Builder': {
>         'recipe_config': 'webrtc_android',
>         'chromium_config_kwargs': {
>           'BUILD_CONFIG': 'Release',
>           'TARGET_PLATFORM': 'android',
>           'TARGET_ARCH': 'arm',
>           'TARGET_BITS': 32,
>         },
>         'bot_type': 'builder',
>         'testing': {'platform': 'linux'},
>         'triggers': [
>           'Android32 Tests (L Nexus5)',
>           'Android32 Tests (L Nexus7.2)',
>         ],
>         'archive_apprtc': True,
>       },

Done.

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
scripts/slave/recipes/webrtc/standalone.py:63:
api.commit_position.parse_revision(webrtc.revision_cp))
On 2016/09/18 17:33:24, kjellander_chromium wrote:
> Instead of this, you can use self.revision_cp if you use the approach
described
> in the next comment (and you'll also be able to remove the commit_position
> dependency of the recipe again).

Done.

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
scripts/slave/recipes/webrtc/standalone.py:64: api.gsutil.upload(zip_path,
'chromium-webrtc', destination,
On 2016/09/18 17:33:24, kjellander_chromium wrote:
> Then we could re-use the existing package_build method and put this step in
> there instead:
>
https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/webrtc/ap...
> 
> Like:
> if self.bot_config.get('archive_apprtc', False):
>   # Do the steps above, but re-use the upload_url variable that already points
> to the <GS bucket>/client.webrtc/Android32 Builder (dbg) directory.
> 
> That way, the recipe stays much more readable.

Moved to the package_build method.

I didn't re-use the upload_url variable. It would have resulted in something
like this:

gs://chromium-webrtc/gs://chromium-webrtc/client.webrtc/Android32
Builder/AppRTCDemo_apk_170242.zip

Because gsutil.upload prefixes 'destination' with 'gs://chromium-webrtc/'.

[kjellander_chromium, 2016-09-19 12:14:24]

lgtm

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
File scripts/slave/recipes/webrtc/standalone.py (right):

https://codereview.chromium.org/2344173002/diff/1/scripts/slave/recipes/webrt...
scripts/slave/recipes/webrtc/standalone.py:64: api.gsutil.upload(zip_path,
'chromium-webrtc', destination,
On 2016/09/19 06:50:37, ehmaldonado_chromium wrote:
> On 2016/09/18 17:33:24, kjellander_chromium wrote:
> > Then we could re-use the existing package_build method and put this step in
> > there instead:
> >
>
https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/webrtc/ap...
> > 
> > Like:
> > if self.bot_config.get('archive_apprtc', False):
> >   # Do the steps above, but re-use the upload_url variable that already
points
> > to the <GS bucket>/client.webrtc/Android32 Builder (dbg) directory.
> > 
> > That way, the recipe stays much more readable.
> 
> Moved to the package_build method.
> 
> I didn't re-use the upload_url variable. It would have resulted in something
> like this:
> 
> gs://chromium-webrtc/gs://chromium-webrtc/client.webrtc/Android32
> Builder/AppRTCDemo_apk_170242.zip
> 
> Because gsutil.upload prefixes 'destination' with 'gs://chromium-webrtc/'.

I see. Looks much better now!

https://codereview.chromium.org/2344173002/diff/20001/scripts/slave/recipes/w...
File
scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_android32_builder.json
(right):

https://codereview.chromium.org/2344173002/diff/20001/scripts/slave/recipes/w...
scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_android32_builder.json:289:
"stdin": "{\"entries\": [{\"archive_name\": null, \"path\":
\"[BUILDER_CACHE]/Android32_Builder/src/out/Release/apks/AppRTCDemo.apk\",
\"type\": \"file\"}], \"output\": \"[SLAVE_BUILD]/AppRTCDemo_apk.zip\",
\"root\": \"[BUILDER_CACHE]/Android32_Builder/src/out/Release/apks\"}"
The root property here looked a bit odd, but it seems to be the same at the iOS
API framework bot, so it should work (I was afraid it expected a relative path
vs the root or something like that).

[ehmaldonado_chromium, 2016-09-19 12:38:03]

The CQ bit was checked by ehmaldonado@chromium.org

[commit-bot: I haz the power, 2016-09-19 12:38:06]

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-09-19 12:45:23]

Description was changed from

==========
Archiving of AppRTC APK in client.webrtc waterfall

To ease manual testing, the AppRTCDemo artifacts from the Android builders are
being archived to Google storage for the following builders:
  Android32 Builder
  Android32 Builder (dbg)
  Android64 Builder
  Android64 Builder (dbg)

BUG=647242
==========

to

==========
Archiving of AppRTC APK in client.webrtc waterfall

To ease manual testing, the AppRTCDemo artifacts from the Android builders are
being archived to Google storage for the following builders:
  Android32 Builder
  Android32 Builder (dbg)
  Android64 Builder
  Android64 Builder (dbg)

BUG=647242

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

[commit-bot: I haz the power, 2016-09-19 12:45:24]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/tools/build/+/a2b056bd69e365ff4c2a...

[kjellander_webrtc, 2016-09-19 15:16:07]

On 2016/09/19 12:45:24, commit-bot: I haz the power wrote:
> Committed patchset #2 (id:20001) as
>
https://chromium.googlesource.com/chromium/tools/build/+/a2b056bd69e365ff4c2a...

Confirming it works!
https://build.chromium.org/p/client.webrtc/builders/Android32%20Builder/build...
I also tried downloading the .zip, which contained the .apk.