GNOME Bugzilla – Bug 712455
GExiv2 needs a test suite.
Last modified: 2017-04-25 16:48:29 UTC
---- Reported by valencia-maint@gnome.bugs 2013-01-02 02:48:00 -0800 ---- Original Redmine bug id: 6166 Original URL: http://redmine.yorba.org/issues/6166 Searchable id: yorba-bug-6166 Original author: Robert Park Original description: Ok, so I discovered that bug in the _delitem_ python method in GExiv2.py, and I realized that GExiv2 really badly needs a test suite. So I wrote one! ;-) Right now I am sitting on a thousand-line-long test suite, that excercises every single method on the GExiv2.Metadata object. I'm just working on being able to run the tests against the code in the source directory (currently the tests are being run against the system-installed version of the library), then I will submit this for review. Related issues: related to gexiv2 - Feature #7660: Port GExiv2 to autotools build system. (Fixed) ---- Additional Comments From valencia-maint@gnome.bugs 2013-11-04 15:40:00 -0800 ---- ### History #### #1 Updated by Jim Nelson 11 months ago * **Target version** set to _0.6_ Fantastic! I would like to start work to get this incorporated as soon as possible. I'm curious, were all 1000-lines hand-coded, or did you use some kind of code generator to create them? #### #2 Updated by Robert Park 11 months ago Mostly hand-coded. As you'll see, there's a few bits that are just checking the presence of long lists of things (methods, exif tags, etc), those were copy&pasted from a terminal and make up maybe 25% of it. But the other 75% was all written by hand by me. I pushed the test suite here if you'd like to preview it: https://github.com/robru/gexiv2-test/blob/master/tests/test_metadata.py Things to note: 1. If you run this as-is, there will be one error, and that's due to the `__delitem__` bug for which I've already submitted a patch... but it's still failing the test because it's testing the system-installed version and not the version in the source tree. Still working on that... 2. Three test methods have 'FIXME_' in the method name. That prevents the test from running, and those tests are disabled because they are currently causing segfaults. The segfaults may be due to incorrect introspection annotations, or may be due to actual bugs in the code. Fortunately these methods seem to be relatively obscure so their failures are relatively low-impact, but it would still be good to clean those up (most likely I'd begin working on those after the test suite is accepted) 4. There's a single #FIXME comment denoting that a method is not returning the value I'd expect it to return, but at least it's not segfaulting. Currently I put the unexpected value into the test, so the test technically passes, but it's something we should look into. Overall I'd say that we are at a fantastically high level of quality for a project that has no test suite, but this can help us find the last few niggles ;-) If you want to run the testsuite yourself, simply `make check` and you'll find that it compiles the library first, then runs the whole test suite twice: once under python2, and once again under python3. The duplication there is necessary because 2 & 3 have enough differences that it's important to ensure they both are working correctly, and that GExiv2 is an important part of the migration path for developers wishing to port their python2/pyexiv2 apps to Python 3, so it's important that we support both pythons fully. This will make it easier for GExiv2 consumers to port their applications from Python 2 to Python 3 without any hiccups. #### #3 Updated by Robert Park 11 months ago Ok, so I have this working under python3 currently but it is broken in python2 for some reason. What I had to do was move `GExiv2.py` into a new subdirectory called `overrides/`, and then I had to add the following code into the test suite: import gi gi.__path__.append('.') This makes the python3 version of the testsuite pass thanks to the source tree version having fixes that aren't installed in the system, but for some reason when I try that under python2, it shows the test failure that makes it obvious that it's using the system GExiv2.py instead of the source tree version. Keeping the tests running under python2 is important to me, so I'll keep working on this... #### #4 Updated by Robert Park 11 months ago * **File** _0001-Add-comprehensive-testsuite.patch_ added * **Category** set to _build_ * **Status** changed from _Open_ to _Review_ * **% Done** changed from _90_ to _100_ * **Resolution** set to _fixed_ Ok, here is the "completed" test suite. Notes: 1. there are still three 'FIXME' disabled tests. I'd like to have the testsuite accepted as-is, and then the future commits that fix those methods can enable those tests to demonstrate that things are working. 2. I still was unable to get python2 to run the testsuite against the binaries in the source tree; however python2 does work fine for testing the system- installed binaries, so I added a new make target for this. Currently, in a fresh source tree you can type `./configure --enable-introspection; make check` and find that python3 runs and finds no errors. Then you can type `make check-installed` and find a single test failure, which is the `__delitem__` for which you've already accepted the patch (this is assuming that you have the 0.5 package installed and haven't installed git master into your system). 3. The test does include a demo image from a real camera for the purposes of reading exif data from, but it has been scaled down considerably in order to avoid having a 5MB binary blob in the git commit history. Anyway, the testsuite may not be perfect but I think we've got a great start here and it can be easily expanded as bugs are found. Once this patch is accepted, no future patches should be accepted unless they include a testcase ;-) #### #5 Updated by Jim Nelson 10 months ago Great! I probably won't have time in the next couple of days to land this, but I will make time as soon as possible. This is exciting indeed. #### #6 Updated by Robert Park 10 months ago * **File** deleted (<strike>_0001-Add-comprehensive-testsuite.patch_</strike>) #### #7 Updated by Robert Park 10 months ago * **File** _0001-Add-comprehensive-testsuite.patch_ added Ok, I've rebased my testsuite on the latest changes in git master. #### #8 Updated by Jim Nelson 9 months ago * **Status** changed from _Review_ to _Open_ * **Resolution** deleted (<strike>_fixed_</strike>) I overlooked this Robert and am now evaluating it. It doesn't currently merge cleanly with trunk (but is easily massaged to make it work). My only problem is when I run "make check" or "make check-installed", it fails every time. Do you see this as well? Also, it looks like this creates a tests/ directory. That should be cleaned up in "make clean". #### #9 Updated by Robert Park 9 months ago * **File** deleted (<strike>_0001-Add-comprehensive-testsuite.patch_</strike>) #### #10 Updated by Robert Park 9 months ago * **File** _0001-Add-comprehensive-testsuite-to-GExiv2.patch_ added Hey Jim, thanks for looking at this. Indeed it does seem as though the patch has bitrotted slightly -- I've now rebased it on latest trunk, and I've also updated `make clean` to remove generated files from the new tests/ directory. I'm not sure what you mean about the test failures though -- can you show me the output specifically of what is failing? Currently `make check` passes completely but `make check-installed` should have a single failure only if the latest stable release is installed in your system, it passes completely if you have latest git master installed. #### #11 Updated by Jim Nelson 9 months ago Thanks for the quick turnaround! I'm still seeing errors, though. Here's what I get with "make check": LD_LIBRARY_PATH=.libs:$LD_LIBRARY_PATH GI_TYPELIB_PATH=.:$GI_TYPELIB_PATH python3 -m unittest discover ERROR:root:Could not find any typelib for GExiv2 E ====================================================================== ERROR: tests.test_metadata (unittest.loader.ModuleImportFailure) ---------------------------------------------------------------------- Traceback (most recent call last): File "/usr/lib/python3.2/unittest/case.py", line 370, in _executeTestPart function() File "/usr/lib/python3.2/unittest/loader.py", line 32, in testFailure raise exception ImportError: Failed to import test module: tests.test_metadata Traceback (most recent call last): File "/usr/lib/python3.2/unittest/loader.py", line 261, in _find_tests module = self._get_module_from_name(name) File "/usr/lib/python3.2/unittest/loader.py", line 239, in _get_module_from_name __import__(name) File "/home/jim/git/gexiv2/tests/test_metadata.py", line 39, in <module> from gi.repository import GExiv2 ImportError: cannot import name GExiv2 ---------------------------------------------------------------------- Ran 1 test in 0.001s FAILED (errors=1) make: *** [check] Error 1 If I install and run "make check-installed": python2 -m unittest discover ERROR:root:Could not find any typelib for GExiv2 E ====================================================================== ERROR: tests.test_metadata (unittest.loader.ModuleImportFailure) ---------------------------------------------------------------------- ImportError: Failed to import test module: tests.test_metadata Traceback (most recent call last): File "/usr/lib/python2.7/unittest/loader.py", line 252, in _find_tests module = self._get_module_from_name(name) File "/usr/lib/python2.7/unittest/loader.py", line 230, in _get_module_from_name __import__(name) File "/home/jim/git/gexiv2/tests/test_metadata.py", line 39, in <module> from gi.repository import GExiv2 ImportError: cannot import name GExiv2 ---------------------------------------------------------------------- Ran 1 test in 0.000s FAILED (errors=1) make: *** [check-installed] Error 1 Does this have something to do with moving Gexiv2.py into overrides? #### #12 Updated by Robert Park 9 months ago No, moving the GExiv2.py file is unrelated (in fact, it was necessary that GExiv2.py live inside a directory called "overrides" in order for me to even get "make check" passing in the first place). If the problem was simply that GExiv2.py file was missing (but the rest of GExiv2 was otherwise installed correctly), you would still be able to import GExiv2 just fine, but you would get this traceback instead: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=697204#17 So that's definitely unrelated to what you're seeing. Your traceback looks more like the typelib file itself is not installed, or perhaps some other piece of the chain that is at a lower level than simply the override file. So when you say you "installed" it, did you install the ubuntu package? or did you compile and install from source? If you just installed the ubuntu package, make sure you install the gir, too (it's in a separate package). So do something like `sudo apt-get install gir.*gexiv` to get what you need. If you installed from source, did you remember to `./configure --enable- introspection` ? Because if you just run `./configure` then the introspection (and thus the test) will be completely missing and broken. Failing those two thoughts, do you have pygobject installed? Try `sudo apt-get install python-gobject python3-gi` and then try running the tests again. Oh, another random thought... when you did `make check`, did you compile first? What happens if you run `make; make check` ? Just wondering if maybe I screwed up the dependency chain in the makefile, resulting in the binaries not getting compiled before trying to run the test suite. #### #13 Updated by Jim Nelson 9 months ago I forgot about --enable-introspection. I re-built and ran "make check" but still had errors: $ make check LD_LIBRARY_PATH=.libs:$LD_LIBRARY_PATH GI_TYPELIB_PATH=.:$GI_TYPELIB_PATH python3 -m unittest discover EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE EEEEEEEEEEEE ====================================================================== ERROR: test_clear (tests.test_metadata.TestMetadata) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/jim/git/gexiv2/tests/test_metadata.py", line 46, in setUp self.metadata = GExiv2.Metadata('tests/data/original.jpg') TypeError: GObject.__init__() takes exactly 0 arguments (1 given) ====================================================================== ERROR: test_clear_exif (tests.test_metadata.TestMetadata) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/jim/git/gexiv2/tests/test_metadata.py", line 46, in setUp self.metadata = GExiv2.Metadata('tests/data/original.jpg') TypeError: GObject.__init__() takes exactly 0 arguments (1 given) ====================================================================== If I install and run "make check-installed", I get this: $ make check-installed python2 -m unittest discover ..E.E..E.......EEEE.E........EFEEE..E...........EFEEEE...E.E..E.......F....E.. E...E...EEE. ====================================================================== ERROR: test_clear_exif_tag (tests.test_metadata.TestMetadata) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/jim/git/gexiv2/tests/test_metadata.py", line 675, in test_clear_exif_tag self.metadata.clear_exif_tag('Exif.Image.Make') AttributeError: 'Metadata' object has no attribute 'clear_exif_tag' ====================================================================== ERROR: test_clear_iptc_tag (tests.test_metadata.TestMetadata) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/jim/git/gexiv2/tests/test_metadata.py", line 889, in test_clear_iptc_tag self.metadata.clear_iptc_tag('Iptc.Application2.City') AttributeError: 'Metadata' object has no attribute 'clear_iptc_tag' ====================================================================== It's okay to require --enable-introspection before "make check", but I think it's worth checking if it's enabled when either make check is run -- if not set, print a warning message. We shouldn't require installing an Ubuntu package, so this should work without the gir package installed. I was running Python 2.7.3. I switched to Python 3.2 and had the same problem. Is it possible you have gir bindings installed already and it's using those instead of the built ones? #### #14 Updated by Robert Park 9 months ago Hmmmm, you're right, seems I had some stuff installed to the system from before that was being pulled in rather than running cleanly from the source dir like I thought. Oddly the error I get from `make check` is the one that you get from `make check-installed`, so it seems like something else is also interfering. The error message that you're seeing for `make check` is indicative of GExiv2.py being missing... I'll have to play with this a bit, sorry. #### #15 Updated by Jim Nelson 8 months ago * **Target version** changed from _0.6_ to _0.7.0_ #### #16 Updated by Jim Nelson about 1 month ago * **Target version** deleted (<strike>_0.7.0_</strike>) #### #17 Updated by Robert Park 14 days ago * **File** deleted (<strike>_0001-Add-comprehensive-testsuite-to-GExiv2.patch_</strike>) #### #18 Updated by Robert Park 14 days ago * **File** 0001-Implement-python-based-testsuite.patch added I am **soooooooooooooooooooo** sorry for dropping the ball on this one for so long. It has bitrotted significantly. So, this is what's up: 1. You went crazy and ripped out massive amounts of API, which caused tests to fail. I've now ripped out tests that were testing missing API calls. 2. I see there's another outstanding patch that ports us to autotools. The old patch massively conflicted with that patch, so for now I've taken special care not to modify the build system with this patch (for now it simply inserts the tests, without any provision for actually running them ever). Once the autotools patch lands I can update this to make the tests run at build time. 3. I've given up on the idea of testing the GExiv2.py overrides, I simply cannot get that to work in a reliable manner. Python insists on it being installed to the system, it can't be tested from within the source tree. It's unfortunate, because it means that the code in the test suite is not reflective of how end users will actually consume the API in python, but at least we have a reliably passing testsuite now. 4. There is still one failing test that I think might actually be a bug in clear_comment() method. grep testsuite for FIXME. That said, while the patch can be cleanly applied now, it is necessarily incomplete until the autotools patch lands. But it's safe to land this patch now if you feel like it, and then we can hook it up in autotools later. To run this test, apply the patch and then run this command: LD_LIBRARY_PATH=.libs:$LD_LIBRARY_PATH GI_TYPELIB_PATH=.:$GI_TYPELIB_PATH python3 -m unittest discover #### #19 Updated by Jim Nelson 11 days ago * **Status** changed from _Open_ to _Review_ * **Target version** set to _0.8.0_ Let's wait for the autotools patch to land, I think it's close. #### #20 Updated by Jim Nelson 11 days ago * **Tracker** changed from _Bug_ to _Feature_ --- Bug imported by chaz@yorba.org 2013-11-16 14:44 UTC --- This bug was previously known as _bug_ 6166 at http://redmine.yorba.org/show_bug.cgi?id=6166 Imported an attachment (id=259982) Unknown milestone "unknown in product gexiv2. Setting to default milestone for this product, "---". Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one. Resolution set on an open status. Dropping resolution
What's the status of the automake transition? Are we ready to start landing my testsuite yet?
Robert, it looks like the autotools patch landed in gexiv2 a couple weeks ago. With gexiv2 on autotools, does your patch still apply, or does it need work to run? (By the way, just to make sure you know, the git repo for gexiv2 has moved to git://git.gnome.org/gexiv2 since this bug last had any activity.) I'm a bit out of the loop here, and Jim's out till next week, so I think we'd wait till next week sometime to land this.
It'll still apply, but it needs work to make autotools actually run the suite. I think I can hook up the bits, will do it this week.
Damn, I completely missed this :-/
Created attachment 350417 [details] [review] Implement python-based testsuite.
Created attachment 350418 [details] [review] Update testsuite to match current test runner
Attachment 350417 [details] pushed as 2498a4a - Implement python-based testsuite. Attachment 350418 [details] pushed as 10adb71 - Update testsuite to match current test runner