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 712455 - GExiv2 needs a test suite.
GExiv2 needs a test suite.
Status: RESOLVED FIXED
Product: gexiv2
Classification: Other
Component: build
unspecified
Other All
: High normal
: ---
Assigned To: Gexiv2 Maintainers
Gexiv2 Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-01-02 10:48 UTC by Valencia Maintainers
Modified: 2017-04-25 16:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Rebased on master as of Nov 1st. (78.04 KB, patch)
2013-11-01 21:42 UTC, Valencia Maintainers
none Details | Review
Implement python-based testsuite. (77.84 KB, patch)
2017-04-25 16:45 UTC, Jens Georg
committed Details | Review
Update testsuite to match current test runner (47.75 KB, patch)
2017-04-25 16:45 UTC, Jens Georg
committed Details | Review

Description Charles Lindsay 2013-11-16 14:44:24 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 

Comment 1 Robert Bruce Park 2013-11-26 08:06:00 UTC
What's the status of the automake transition? Are we ready to start landing my testsuite yet?
Comment 2 Charles Lindsay 2013-11-26 20:37:16 UTC
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.
Comment 3 Robert Bruce Park 2013-11-26 22:43:21 UTC
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.
Comment 4 Jens Georg 2017-04-24 10:41:20 UTC
Damn, I completely missed this :-/
Comment 5 Jens Georg 2017-04-25 16:45:39 UTC
Created attachment 350417 [details] [review]
Implement python-based testsuite.
Comment 6 Jens Georg 2017-04-25 16:45:45 UTC
Created attachment 350418 [details] [review]
Update testsuite to match current test runner
Comment 7 Jens Georg 2017-04-25 16:48:21 UTC
Attachment 350417 [details] pushed as 2498a4a - Implement python-based testsuite.
Attachment 350418 [details] pushed as 10adb71 - Update testsuite to match current test runner