GNOME Bugzilla – Bug 699601
Add --enable-installed-tests, use it for reftests
Last modified: 2013-05-09 17:14:35 UTC
See https://live.gnome.org/GnomeGoals/InstalledTests --- configure.ac | 6 ++++++ tests/reftests/Makefile.am | 39 +++++++++++++++++++++++++++++++++------ tests/reftests/gtk-reftest.c | 2 +- 3 files changed, 40 insertions(+), 7 deletions(-)
Created attachment 243215 [details] [review] 0001-reftests-Use-pixbuf-property-for-images-to-be-relati.patch
Created attachment 243216 [details] [review] 0001-Add-enable-installed-tests-use-it-for-reftests.patch
(In reply to comment #1) > Created an attachment (id=243215) [details] [review] > 0001-reftests-Use-pixbuf-property-for-images-to-be-relati.patch I'm using the file property on purpose in that test, because the file property goes through gdk_pixbuf_animation_new_from_file() while the pixbuf property goes through gdk_pixbuf_new_from_file(), and the test is mostly testing brokenness in gdk-pixbuf (well, and making sure the file property works, is a good idea, too). Can and should we instead maybe set the current working directory to the directory the test is loaded from?
Review of attachment 243216 [details] [review]: ::: tests/reftests/Makefile.am @@ +36,3 @@ + button-wrapping.ui \ + button-wrapping.ref.ui \ + $(NULL) I'd rather fix that test than anyone putting anything xfail in Makefiles. (Yes, I committted that test prematurely, sorry) I'd rather you git revert the patch committing the test than having an xfail. ::: tests/reftests/gtk-reftest.c @@ +584,3 @@ basedir = g_getenv ("srcdir"); else + basedir = INSTTESTDIR; This breaks uninstalled ./gtk-reftest though, doesn't it? And I think that's something I'd like to keep working. Not sure if gtk-reftest can detect if it's installed or not? If not, it might be beneficial to just install a gtk-reftest-runner script that runs srcdir="/path/to/installed" gtk-reftest or so?
(In reply to comment #3) > (In reply to comment #1) > > Created an attachment (id=243215) [details] [review] [details] [review] > > 0001-reftests-Use-pixbuf-property-for-images-to-be-relati.patch > > I'm using the file property on purpose in that test, because the file property > goes through gdk_pixbuf_animation_new_from_file() while the pixbuf property > goes through gdk_pixbuf_new_from_file(), and the test is mostly testing > brokenness in gdk-pixbuf (well, and making sure the file property works, is a > good idea, too). Hmmm...is there that much of a difference? Don't we have test coverage of those elsewhere? > Can and should we instead maybe set the current working directory to the > directory the test is loaded from? The current InstalledTests specification says that each test is given a clean empty temporary directory that is automatically cleaned up, mainly because for a lot of tests that's very useful. Now while we *could* change the cwd, the problem with that is that we need to output the .diff.png and such to the test tempdir, because the test doesn't have write access to the directory it's contained in (/usr/libexec/gtk+/installed-tests/reftests is owned by root).
And it'd obviously be nice to say keep around the tmpdir on fail, so any lookaside artifacts like these PNG files could be viewed. But I can't do that unless the tests write into a well known directory. Maybe we could add gtk_builder_set_base_directory() or something?
Created attachment 243248 [details] [review] 0001-Revert-tests-Add-reftest-for-broken-button-sizing.patch
Created attachment 243249 [details] [review] 0001-Add-enable-installed-tests-use-it-for-reftests.patch
I've reworked the patches a bit with a bunch of IRC discussions with involved people and pushed them to master. If anything's still borked we can fix it there.