605541 - add buildstep to output tinderboxprint of changeset details

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Mike Taylor [:bear]]

The tinderbox display is missing mobile and mozilla changeset details, this patch adds the build step to bring the tegra waterfall display into line with the others.

Comment 1

[Mike Taylor [:bear]]

Attachment: add tinderboxprint build step to talos factory for remote tests

Comment 2

[Aki Sasaki (not active)]

Comment on attachment 484386 [details] [diff] [review]
add tinderboxprint build step to talos factory for remote tests

>         self.addStep(SetProperty,
>          command=['cat', WithProperties('%(exedir)s/application.ini')],
>          workdir=os.path.join(self.workdirBase, "talos"),
>          extract_fn=get_build_info,
>          name='get build info',
>         )

Noticed that you noticed that this is already giving you the mobile info; cool.

>+        if self.remoteTests:

This assumes that all remoteTests will be Fennec, which is sadly an invalid assumption... we may test Firefox via SUT at some point, on Tegra or some netbook-y platform.

I don't think the is_mobile flag is passed down to TalosFactory.
Currently our hardcoded fix to detect mobile is to check if self.productName == 'fennec'.
(This introduces its own set of problems, but we've already hardcoded Fennec a number of places, so we're only incrementally additionally screwed here.)

Therefore I'd change this to |if self.productName == 'fennec':| or |if self.remoteTests and self.productName == 'fennec':|
Probably the former, so we have the option of using TalosFactory later on for n900s, which are fennec but not remote.

>+            self.addStep(ShellCommand(
>+                command=['echo', 'TinderboxPrint:',
>+                         WithProperties('<a href=%(mozilla_repository)s/rev/%(mozilla_changeset)s' +
>+                                        'title="Built from Mozilla revision %(mozilla_changeset)s">' +
>+                                        'moz:%(mozilla_changeset)s</a> <br />' +
>+                                        '<a href=%(repo_path)s/rev/%(revision)s' +
>+                                        'title="Built from Mobile revision %(revision)s">' +
>+                                        'mobile:%(revision)s</a>')],
>+                description=['list', 'revisions'],
>+                name='rev_info',
>+            ))

Ok. you're going to re-introduce a whitespace bug:

<a href=http://blah/rev/blahtitle="Built from Mozilla revision blah"> ...

basically you need a space after the href=link and the ' for both mozilla_changeset and revision.

This looks pretty safe, but we are running in production.
If you feel confident in this, I'll r+ after you add the two spaces in the TinderboxPrint and change the |if self.remoteTests:|, but if you're not sure, feel free to let this bake in your dev/staging env for a while.  If you're using tip of buildbot-configs/buildbotcustom:buildbot-0.8.0 branch, then you should be sending your results to the MobileTest tinderbox tree and should see the changeset links there.

Comment 3

[Mike Taylor [:bear]]

Attachment: add tinderboxprint build step to talos factory for remote tests

added extra level of check to make sure the change is applied only to android remote tests and also fixed whitespace issue with anchor tag

Comment 4

[Mike Taylor [:bear]]

Attachment: add tinderboxprint build step to talos factory for remote tests

Comment 5

[Mike Taylor [:bear]]

committed changeset 1009:a299f4a92e90

Comment 6

[Aki Sasaki (not active)]

Fixed :)