After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 749593 - Miscellaneous test fixes
Miscellaneous test fixes
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2015-05-19 17:34 UTC by Cosimo Cecchi
Modified: 2015-05-24 19:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
parse-sass: don't hardcode bash path (709 bytes, patch)
2015-05-19 17:34 UTC, Cosimo Cecchi
needs-work Details | Review
Use built-in gtk-update-icon-cache (2.02 KB, patch)
2015-05-19 17:34 UTC, Cosimo Cecchi
reviewed Details | Review
tests: wait for draw before fetching tree view style (969 bytes, patch)
2015-05-19 17:35 UTC, Cosimo Cecchi
accepted-commit_now Details | Review
parse-sass: don't hardcode bash path (695 bytes, patch)
2015-05-21 23:47 UTC, Cosimo Cecchi
accepted-commit_now Details | Review
Use built-in gtk-update-icon-cache (2.17 KB, patch)
2015-05-21 23:48 UTC, Cosimo Cecchi
accepted-commit_now Details | Review

Description Cosimo Cecchi 2015-05-19 17:34:49 UTC
See attached patches.
Comment 1 Cosimo Cecchi 2015-05-19 17:34:54 UTC
Created attachment 303615 [details] [review]
parse-sass: don't hardcode bash path

It's /bin/bash on some systems; use /usr/bin/env to find out.
Comment 2 Cosimo Cecchi 2015-05-19 17:34:57 UTC
Created attachment 303616 [details] [review]
Use built-in gtk-update-icon-cache

To generate the icon cache files.
Some systems rename these files on installation, to avoid conflicts with
the GTK2 versions. Just use the ones we built in-tree to avoid
problems.
Comment 3 Cosimo Cecchi 2015-05-19 17:35:00 UTC
Created attachment 303617 [details] [review]
tests: wait for draw before fetching tree view style

On some slower machines (e.g. an ARM OBS builder), this test is failing
with a race condition where we're trying to fetch the style before it's
applied.
Comment 4 Matthias Clasen 2015-05-21 01:44:04 UTC
Review of attachment 303615 [details] [review]:

Come on, thats ridiculous.
Just make it

#! /bin/sh
Comment 5 Matthias Clasen 2015-05-21 01:46:33 UTC
Review of attachment 303617 [details] [review]:

hmm, ok
Comment 6 Matthias Clasen 2015-05-21 01:47:56 UTC
Review of attachment 303616 [details] [review]:

but what are the problems ? if they rename them to avoid conflict with an already-installed gtk-update-icon-cache, we'll just use that already-installed one...
Comment 7 Cosimo Cecchi 2015-05-21 23:47:53 UTC
Created attachment 303787 [details] [review]
parse-sass: don't hardcode bash path

It's /bin/bash on some systems; just use /bin/sh instead.
Comment 8 Cosimo Cecchi 2015-05-21 23:48:02 UTC
Created attachment 303788 [details] [review]
Use built-in gtk-update-icon-cache

To generate the icon cache files.

We want to avoid a dependency loop if possible; additionally, on some
Debian-based systems gtk-update-icon-cache maps to the GTK2 version of
the utility and the GTK3 version is renamed to
gtk-update-icon-cache-3.0.

To avoid a build dependency on GTK2, use the binary that we just built
in-tree.
Comment 9 Cosimo Cecchi 2015-05-21 23:49:19 UTC
Thanks for the review Matthias.
I fixed the shebang line in parse-sass to reference /bin/sh and expanded the commit message for the gtk-update-icon-cache patch to include some more rationale on why I think this is a good idea.
Comment 10 Matthias Clasen 2015-05-22 03:48:58 UTC
Review of attachment 303787 [details] [review]:

ok
Comment 11 Matthias Clasen 2015-05-22 03:53:32 UTC
Review of attachment 303788 [details] [review]:

I was wondering if this would be a problem for cross builders, but I guess we're already skipping that part here via test - n "$(DESTDIR)". Are you not doing DESTDIR builds ?
Comment 12 Cosimo Cecchi 2015-05-24 19:16:05 UTC
I think builds of the debian packages by the OBS builder will be DESTDIR, but the packaging will also make sure to separately trigger an icon cache update when the package is installed on the machine.
That patch was needed to fix failures in a Jenkins continuous integration builder that basically just runs make distcheck to prepare the sources for the debian OBS builder.
Comment 13 Cosimo Cecchi 2015-05-24 19:16:31 UTC
Pushed these to master, thanks.

Attachment 303617 [details] pushed as cfbd862 - tests: wait for draw before fetching tree view style
Attachment 303787 [details] pushed as 02be889 - parse-sass: don't hardcode bash path
Attachment 303788 [details] pushed as 345f2a4 - Use built-in gtk-update-icon-cache