Code review comment for lp:~rick-rickspencer3/lazr.restfulclient/add_gaurd_for_None_to_eq_functions

Revision history for this message
Leonard Richardson (leonardr) wrote :

This is a good fix, but I won't land a branch without tests. I'm not sure what you mean by "the tests for docs were not very complete." My best guess is that you found the test fixtures confusing and couldn't locate the doctests.

An example should help you see how the test system works. To test your two changes I added tests to src/lazr/restfulclient/docs/entries.txt (section "Comparing entries") and src/lazr/restfulclient/docs/hosted-files.txt (section "Comparing hosted files"). My diff is below.

Since this branch now contains my code I'm going to get someone else to review it, and then I'll land it.

=== modified file 'src/lazr/restfulclient/NEWS.txt'
--- src/lazr/restfulclient/NEWS.txt 2009-12-15 12:28:41 +0000
+++ src/lazr/restfulclient/NEWS.txt 2009-12-17 14:55:45 +0000
@@ -5,6 +5,7 @@
 Development (XXXX-XX-XX)
 ========================
 - Added a proof-of-concept test for OAuth-signed anonymous access.
+- Fixed comparisons of entries and hosted files with None.

 0.9.10 (2009-10-23)
 ===================

=== modified file 'src/lazr/restfulclient/docs/entries.txt'
--- src/lazr/restfulclient/docs/entries.txt 2009-10-21 12:12:44 +0000
+++ src/lazr/restfulclient/docs/entries.txt 2009-12-17 14:41:24 +0000
@@ -402,6 +402,11 @@
     >>> recipe == another_recipe
     False

+An entry is never equal to None.
+
+ >>> recipe == None
+ False
+
 If one entry represents the current state of the server, and the other
 is out of date or has client-side modifications, they will not be
 considered equal.

=== modified file 'src/lazr/restfulclient/docs/hosted-files.txt'
--- src/lazr/restfulclient/docs/hosted-files.txt 2009-10-21 12:12:44 +0000
+++ src/lazr/restfulclient/docs/hosted-files.txt 2009-12-17 14:51:41 +0000
@@ -89,7 +89,6 @@
 Two hosted file objects are the same if they point to the same
 server-side resource.

-
     >>> cover = service.cookbooks['Everyday Greens'].cover
     >>> cover_2 = service.cookbooks['Everyday Greens'].cover
     >>> cover == cover_2
@@ -99,6 +98,11 @@
     >>> cover == other_cover
     False

+A hosted file is never equal to None.
+
+ >>> cover == None
+ False
+
 Error handling
 --------------

=== modified file 'src/lazr/restfulclient/resource.py'
--- src/lazr/restfulclient/resource.py 2009-10-23 13:22:44 +0000
+++ src/lazr/restfulclient/resource.py 2009-12-17 14:43:27 +0000
@@ -365,6 +365,8 @@
         retrieve or modify the hosted file contents is to open a
         filehandle, which goes direct to the server.
         """
+ if other is None:
+ return False
         return self._wadl_resource.url == other._wadl_resource.url

@@ -600,6 +602,7 @@
         contain the same values.
         """
         return (
+ other is not None and
             self.self_link == other.self_link and
             self.http_etag == other.http_etag and
             self._dirty_attributes == other._dirty_attributes)

« Back to merge proposal