Write automated tests for pydigg.

[GoogleCodeExporter]

Write automated tests using unittest, nose, or py.test, for pydigg:

      http://code.google.com/p/pydigg/

Write the tests to achieve > 70% code coverage (see RecordingCodeCoverage).

Completion:

Attach an 'svn diff' (context diff) against the latest pydigg source to this
task and send it to the author of pydigg.

Relevant wiki pages:

 - HowToTest
 - RecordingTestCoverage

Task duration: please complete this task within 5 days (120 hours) of claiming 
it.

Original issue reported on code.google.com by the.good...@gmail.com on 26 Nov 2007 at 6:27

[GoogleCodeExporter]

Original comment by the.good...@gmail.com on 27 Nov 2007 at 8:39

• Added labels: test
• Removed labels: tests

[GoogleCodeExporter]

I claim this task. Pydigg code looks very sweet :)

Original comment by turkay.e...@gmail.com on 29 Nov 2007 at 7:31

[GoogleCodeExporter]

Original comment by doug.hel...@gmail.com on 29 Nov 2007 at 7:45

• Changed state: Claimed
• Added labels: ClaimedBy-turkay.eren

[GoogleCodeExporter]

Hello,

I've done Pydigg unittests. I tested almost all methods and found 2 major 
problems
that I couldn't fix, and 4 minor problems I managed to fix.

First, digg.getStoryDigg() can't be used, when I try to use it, it gives error;

>>> d.getStoryDiggs(777)
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "digg.py", line 71, in getStoryDiggs
    return self._parseDiggs(data)
  File "digg.py", line 956, in _parseDiggs
    events.append(Digg.Digg(self, digg.date, digg.story, digg.id, digg.user,
digg.status))
AttributeError: Bag instance has no attribute 'user'

Even if the story doesn't exist, you shouldn't face this problem but rather
Digg.Error. (There isn't any Bag class)

Second, when using digg.getPopularStories(), -if you give 0 value for sort
parameter-, digg website returns Internal Server Error which is not related with
code. Digg website should return something like "Invalid sort parameter".

>>> d.getPopularStories('', '', '', 0)
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "/home/mini/works/python/pydigg/digg.py", line 226, in getPopularStories
    data = self._get('/stories/popular',min_submit_date=str(min_submit_date),
max_submit_date=str(max_submit_date),min_promote_date=str(min_promote_date),max_
promote_date=str(max_promote_date),
sort=sort,count=str(count),offset=str(offset),domain=domain,link=link)
  File "/home/mini/works/python/pydigg/digg.py", line 1085, in _get
    raise Digg.Error(data.error.code, data.error.message)
digg.Error: [500] Internal error

Minor Problems

I made Digg.Error class printable. When an exception is occured and you don't 
handle
it, python interpter just writes "Error: <unprintable object", it makes 
debugging
hard. But with this fix, it writes "Error: [1005] Error Message here" ;)

I removed "len(value) > 0" statement in _get() because when there is an 
uncountable
data, an error exception occurs. The code just controls whether a value got or 
not,
we don't need to count it to determine if there is.

Also, I added str() functions to a lot of methods. You can pass an integer 
value to
these functions, but when you pass, ValueError occurs since the code looks like 
this; 

self._get('/stories/' + story_ids + /list .........

python can't append integer to string, so they should be string by default.

[~/works/python/pydigg]> python testcase.py
...............................................................................
----------------------------------------------------------------------
Ran 79 tests in 238.606s
OK

Could you review it?

Original comment by turkay.e...@gmail.com on 1 Dec 2007 at 6:38

Attachments:

• pydigg-minor-fixes-and-unittest.patch

[GoogleCodeExporter]

Thanks, Turkay.  I believe Grig is reviewing this now; ping me if he doesn't 
get back
to you.  The unit tests look good to me, though.

Have you contacted the pydigg author with the problems you ran into?

Original comment by the.good...@gmail.com on 2 Dec 2007 at 7:34

[GoogleCodeExporter]

Arrgh.  That would be 'Eren'.  Sorry.

Original comment by the.good...@gmail.com on 2 Dec 2007 at 7:34

[GoogleCodeExporter]

Yes, I mailed to inform pydigg developer and gave this issue's URL yesterday. He
haven't contacted to me yet.

Original comment by turkay.e...@gmail.com on 2 Dec 2007 at 7:53

[GoogleCodeExporter]

Original comment by the.good...@gmail.com on 2 Dec 2007 at 8:13

• Changed state: Completed

[GoogleCodeExporter]

Original comment by the.good...@gmail.com on 3 Dec 2007 at 7:15

• Added labels: FollowupBy-titus

[GoogleCodeExporter]

Hi, Eren

I think you did a good job on writing the unit tests. You certainly tried to be 
very
thorough and test all methods of the original module, so kudos for that. Also, 
the
code coverage is very good. I ran figleaf on your testcase.py test file and I 
got
82.7% (my figleaf HTML report is here: <http://unisonis.com/ghop/153/>).

My general observation is that you have a lot of tests for negative conditions, 
which
is really good and necessary (these are sometimes called 'sad paths' through the
application; they specify what shouldn't happen in your app). You don't have 
that
many unit tests for 'happy paths' though -- things that should happen in your 
app. 

Usually, when you write a unit test, you send a known input to a function/method
under test, and you expect certain things from the output. In your case, I 
noticed
you did have API_KEY, USERNAME, STORY_TITLE and STORY_ID as the known inputs, 
but I
didn't see many assertions about the expected outputs. To take the user name for
example, here's what I would have expected to see (in doctest format):

[ggheo@concord 153]$ python2.5
Python 2.5 (r25:51908, Feb 12 2007, 10:43:09) 
[GCC 3.2.2 20030222 (Red Hat Linux 3.2.2-5)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from digg import *
>>> API_KEY="http://www.testing-pydigg.org"
>>> USERNAME="supernova17"
>>> d = Digg(API_KEY)
>>> u = d.getUserDiggs(USERNAME)
>>> stories = u.getStories()
>>> s = stories[0]
>>> s.title
u'NIST Creates Perpetual Motion ... But Only for 10 Seconds'
>>> s.user_name
u'TheAttacks'
>>> s.id
u'4337169'

What the above snippet of code does is it sends the known USERNAME to the
getUserDiggs method, then it calls certain methods on the object it returns. If 
you
were to write doctests, this is all you'd need to do, but for unittest-based 
tests,
you'd need to assert specifically that you got the output above.

Contrast this with the test you wrote:

    def test_user_diggs(self):
        # if user haven't sent anything, also control that it returns nothing
        digg = self.digg.getUserDiggs(USERNAME)
        if len(digg) > 0:
            # this should be Digg.Digg instance, because it returns Digg object which
we tested before
            self.assert_(isinstance(digg[0], Digg.Digg))

You're not actually asserting anything in your test against a known output, so 
you
don't know exactly what was returned, other than that it was an instance of the
Digg.Digg class. And many of your test methods just use isinstance to assert 
that a
certain object belongs to a certain class. This is not necessarily bad, but it's
overkill. If you take the approach I recommended and verify outputs against 
known
values, you implicitly verify that the return objects do what you expect of 
them.

Also, the unit test methods should have as little logic as possible, so I 
recommend
not using if statement within such methods. If you find yourself reaching for an
'if', consider splitting the unit test method into multiple different methods, 
each
testing a specific branch of the if.

As Titus also said, I think it would be a great idea to contact the author and 
send
him your patches, along with your unit tests. 

Overall, you did a great job, and I hope you'll continue to keep testing in 
mind when
you're writing code :-)

Grig

Original comment by grig.ghe...@gmail.com on 3 Dec 2007 at 7:40

[GoogleCodeExporter]

Copied from
http://groups.google.com/group/ghop-python/browse_thread/thread/26b56ed351749f63
#cea7508133443057

Thanks a lot for suggestions.

You're right, I controlled negative conditions much because I don't have digg
account and I don't want to deal with sending diggs just for testing. If I
controllled known output for random user, one day, a user could change the
context and test would fail, that's why I didn't write.

I suppose that pydigg developer or anyone else has digg account and send a
context that will not be replaced/modified. It won't be hard to control known
output for known user since testcase is understandable enough to add new
functions.

Note: I'll get a digg account and update the patch when I'm done with
SimpleXMLRPCServer testcase.

Original comment by turkay.e...@gmail.com on 4 Dec 2007 at 3:20