Bug 78495 - Flakiness dashboard should show Linux ChromiumOS test results.
Summary: Flakiness dashboard should show Linux ChromiumOS test results.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-13 06:47 PST by Robert Flack
Modified: 2012-02-14 14:44 PST (History)
2 users (show)

See Also:


Attachments
Patch to add Chromium.ChromiumOS master and relevant builders to flakiness dashboard. (3.46 KB, patch)
2012-02-13 06:49 PST, Robert Flack
no flags Details | Formatted Diff | Diff
Patch (4.05 KB, patch)
2012-02-13 10:36 PST, Robert Flack
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Flack 2012-02-13 06:47:02 PST
The ChromiumOS waterfall at http://build.chromium.org/p/chromium.chromiumos/console links to the flakiness dashboard for failed Linux ChromiumOS tests. These tests use the same infrastructure as the Chromium tests and are for the most part a superset of the Chromium tests (Chromium tests + ChromiumOS tests). We should be able to view these test failures on the flakiness dashboard as with any other Chromium tests.
Comment 1 Robert Flack 2012-02-13 06:49:53 PST
Created attachment 126767 [details]
Patch to add Chromium.ChromiumOS master and relevant builders to flakiness dashboard.
Comment 2 Ojan Vafai 2012-02-13 09:58:48 PST
Comment on attachment 126767 [details]
Patch to add Chromium.ChromiumOS master and relevant builders to flakiness dashboard.

View in context: https://bugs.webkit.org/attachment.cgi?id=126767&action=review

Just a couple style nits.

This patch needs a ChangeLog file entry before it can be committed. I recommend uploading the patch using "Tools/Scripts/webkit-patch upload 78495 ".  That will automatically generate the ChangeLog entry for you and upload it here. Notably it will also set the r? flag for you which marks this as a patch needing review.

> Tools/TestResultServer/static-dashboards/builders.js:44
> +CHROMIUM_CHROMIUMOS_BUILDER_MASTER = new BuilderMaster('ChromiumChromiumOS', 'http://build.chromium.org/p/chromium.chromiumos/builders/');

Nit: The Chromium is clear from ChromiumOS, can we call these CHROMIUMOS_BUILDER_MASTER and 'ChromiumOS'?

> Tools/TestResultServer/static-dashboards/dashboard_base.js:181
> +            function() { return value in LAYOUT_TESTS_BUILDER_GROUPS ||
> +                                value in CHROMIUM_GTESTS_BUILDER_GROUPS; });

I'd either make this a single-line or break it up into normal function format like so:
function() {
    return foo ||
        bar;
}
Comment 3 Robert Flack 2012-02-13 10:36:12 PST
Created attachment 126792 [details]
Patch
Comment 4 Robert Flack 2012-02-13 10:44:15 PST
(In reply to comment #2)
> (From update of attachment 126767 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=126767&action=review
> 
> Just a couple style nits.
> 
> This patch needs a ChangeLog file entry before it can be committed. I recommend uploading the patch using "Tools/Scripts/webkit-patch upload 78495 ".  That will automatically generate the ChangeLog entry for you and upload it here. Notably it will also set the r? flag for you which marks this as a patch needing review.
> 
> > Tools/TestResultServer/static-dashboards/builders.js:44
> > +CHROMIUM_CHROMIUMOS_BUILDER_MASTER = new BuilderMaster('ChromiumChromiumOS', 'http://build.chromium.org/p/chromium.chromiumos/builders/');
> 
> Nit: The Chromium is clear from ChromiumOS, can we call these CHROMIUMOS_BUILDER_MASTER and 'ChromiumOS'?
> 

I changed the variable to CHROMIUMOS_BUILDER_MASTER. As for the master name, the master on test-results.appspot.com is called ChromiumChromiumOS, if I change this here I would have to map it back to ChromiumChromiumOS when building the request URL for test results. I think to change the actual master name would require many changes elsewhere.

> > Tools/TestResultServer/static-dashboards/dashboard_base.js:181
> > +            function() { return value in LAYOUT_TESTS_BUILDER_GROUPS ||
> > +                                value in CHROMIUM_GTESTS_BUILDER_GROUPS; });
> 
> I'd either make this a single-line or break it up into normal function format like so:
> function() {
>     return foo ||
>         bar;
> }

Done.
Comment 5 Ojan Vafai 2012-02-13 10:45:21 PST
Comment on attachment 126792 [details]
Patch

Thanks for doing this. I'm also setting cq+ so that this goes through the commit queue. For future reference, you need to set cq? to tell the reviewer you'd like it in the commit queue once it's been reviewed.

Once this patch is committed, it'll need to be pushed to the appengine server. I can do that later today.
Comment 6 WebKit Review Bot 2012-02-13 21:19:38 PST
Comment on attachment 126792 [details]
Patch

Clearing flags on attachment: 126792

Committed r107667: <http://trac.webkit.org/changeset/107667>
Comment 7 WebKit Review Bot 2012-02-13 21:19:42 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Ojan Vafai 2012-02-14 14:44:47 PST
I've pushed this to appengine. Looks like it's working. http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40DEPS%20CrOS%20-%20chromium.org&testType=ui_tests