Bug 30223 - [LayoutTests][Gtk] Set a common Gtk theme as default and update the results
: [LayoutTests][Gtk] Set a common Gtk theme as default and update the results
Status: RESOLVED FIXED
: WebKit
Tools / Tests
: 528+ (Nightly build)
: PC Linux
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-10-08 10:54 PST by
Modified: 2009-10-15 13:37 PST (History)


Attachments
Set Raleigh as the default theme (1.32 KB, patch)
2009-10-08 11:13 PST, Zan Dobersek
eric: review+
eric: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Set Raleigh as the default theme (1.19 KB, patch)
2009-10-09 00:49 PST, Zan Dobersek
zecke: review+
Review Patch | Details | Formatted Diff | Diff
Updated results (4.48 KB, patch)
2009-10-11 10:46 PST, Zan Dobersek
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-10-08 10:54:28 PST
At the moment, unexpected failures can occur because of running the tests while having different Gtk themes set as default. This appears on the Gtk buildbots, where tests are run under Xvfb, in which Raleigh is used as the default theme. Running the tests with a different theme can produce failures.

The Raleigh theme is also Gtk's default and is as such present on every Gtk system. Therefor, I recommend setting this theme as default in the DumpRenderTree.

This bug should be considered for closing when a default theme is set and the results are updated to reflect the changes.
------- Comment #1 From 2009-10-08 11:13:59 PST -------
Created an attachment (id=40894) [details]
Set Raleigh as the default theme

Sets Raleigh as the default theme.
------- Comment #2 From 2009-10-08 12:06:05 PST -------
(From update of attachment 40894 [details])
Sounds fine to me.  Seems silly to wrap this line when lines just below it have no wrapping.  WebKit style has no official column width.

Also WebKit style is to use 0 instead of NULL for c++ code, although maybe the gtk code follows some other style?  check-webkit-style should help identify some style issues.

looks fine, but cq- for the style oddities.
------- Comment #3 From 2009-10-08 12:22:51 PST -------
Maybe consider setting the font names and sizes as well?  But maybe that's a different bug.  If you're really loading all of this from GtkSettings you'll want to override all of 'em, including cursor-blink etc.

(Is this committable without test rebaselines?  Or are they all baselined to Raleigh?)
------- Comment #4 From 2009-10-09 00:35:50 PST -------
(In reply to comment #2)
> (From update of attachment 40894 [details] [details])
> 
> Also WebKit style is to use 0 instead of NULL for c++ code, although maybe the
> gtk code follows some other style?  check-webkit-style should help identify
> some style issues.
> 

I'll raise a discussion on the irc channel about 0 vs NULL problem. I'll also see about opening a bug for style cleanup of Gtk port's code.

(In reply to comment #3)
> Maybe consider setting the font names and sizes as well?  But maybe that's a
> different bug.  If you're really loading all of this from GtkSettings you'll
> want to override all of 'em, including cursor-blink etc.

Font families and sizes are taken care of in resetDefaultsToConsistentValues function, which is called before each test. I don't think overriding any other setting would do any good, or harm.

If it, however, appears that it should be done, the code should be updated - moving all of the overrides of GtkSettings into a new function for a better overview.

> (Is this committable without test rebaselines?  Or are they all baselined to
> Raleigh?)

Rebaselining patch will be attached to this bug asap.
------- Comment #5 From 2009-10-09 00:49:37 PST -------
Created an attachment (id=40935) [details]
Set Raleigh as the default theme

Replaces use of wrapped lines.
------- Comment #6 From 2009-10-10 05:46:43 PST -------
(From update of attachment 40935 [details])
Using NULL as sentinel is what we seem to do now and I see little reason to change that. Could you please watch the bot when landing it?
------- Comment #7 From 2009-10-11 10:46:16 PST -------
Created an attachment (id=41001) [details]
Updated results

This updates results for two tests that were failing on the buildbots because results were generated in a theme different than Raleigh.
------- Comment #8 From 2009-10-13 18:41:28 PST -------
(From update of attachment 41001 [details])
sure.
------- Comment #9 From 2009-10-13 20:37:33 PST -------
(From update of attachment 41001 [details])
Clearing flags on attachment: 41001

Committed r49552: <http://trac.webkit.org/changeset/49552>
------- Comment #10 From 2009-10-13 20:37:37 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #11 From 2009-10-15 13:37:15 PST -------
(From update of attachment 40935 [details])
Removing cq? on this closed bug.  I assume this patch was already landed.