GTG

Merge lp:~jml/gtg/more-tests into lp:~gtg/gtg/old-trunk

  1. more-tests
  2. Merge into old-trunk
Proposed by Jonathan Lange
Status: Merged
Merged at revision: not available
Proposed branch: lp:~jml/gtg/more-tests
Merge into: lp:~gtg/gtg/old-trunk
Diff against target: None lines
To merge this branch: bzr merge lp:~jml/gtg/more-tests
Reviewer Review Type Date Requested Status
Gtg developers Pending
Review via email: mp+8933@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

The main purpose of this branch is to add tests for the Tag class.

The tests that I've added were designed to verify the current behaviour of the class. All of the tests pass running against the version of Tag that's in trunk. Two of the tests check behaviour that looks to me to be accidental, but I figured I'd add them and let someone who knows more than me make an informed decision.

Anyway, once the tests were written I took the opportunity to:
  a) Add docstrings for Tag
  b) Clean up whitespace
  c) Fix PEP 8 violations (remove spaces before colons, add spaces after commas, drop haskey usage)
  d) Changed the internal attributes to have underscore prefixes.
  e) Removed the "defensive" thing in get_all_attributes, since if ever 'name' isn't an attribute, it's actually a serious error.
  f) Changed set_attribute so that 'name' is slightly less special cased.

I think all of these changes are good ones, but tastes may vary.

jml

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'GTG/core/tagstore.py'
2--- GTG/core/tagstore.py 2009-03-31 09:29:10 +0000
3+++ GTG/core/tagstore.py 2009-07-17 11:39:38 +0000
4@@ -149,45 +149,71 @@
5 #########################################################################
6 ######################### Tag ###########################################
7
8-#A tag is defined by its name (in most cases, it will be "@something") and it can have multiple attributes
9-class Tag :
10-
11- def __init__(self,name,save_cllbk=None) :
12- self.attributes = {}
13- self.name = name
14- self.set_attribute("name",self.name)
15- self.save = save_cllbk
16-
17- def get_name(self) :
18+class Tag:
19+ """A short name that can be applied to Tasks.
20+
21+ I mean, surely you must know what a tag is by now. Think Gmail,
22+ del.icio.us, Flickr et al.
23+
24+ A tag is defined by its name, which in most cases is '@something'. A tag
25+ can also have multiple arbitrary attributes. The only attribute enforced
26+ for tags is 'name', which always matches `Tag.get_name()`.
27+ """
28+
29+ def __init__(self, name, save_cllbk=None):
30+ """Construct a tag.
31+
32+ :param name: The name of the tag. Should be a string, generally a
33+ short one.
34+ :param save_cllbk: A nullary callable, called whenever an attribute
35+ is set.
36+ """
37+ self._name = str(name)
38+ self._attributes = {'name': self._name}
39+ self._save = save_cllbk
40+
41+ def get_name(self):
42+ """Return the name of the tag."""
43 return self.get_attribute("name")
44-
45- def set_attribute(self,att_name,att_value) :
46- #warning : only the constructor can set the "name"
47- if att_name != "name" :
48- #Attributes should all be strings
49- val = unicode(str(att_value),"UTF-8")
50- self.attributes[att_name] = val
51- self.save()
52- elif self.name == att_value :
53- self.attributes[att_name] = str(att_value)
54-
55- def get_attribute(self,att_name) :
56- if self.attributes.has_key(att_name) :
57- return self.attributes[att_name]
58- else :
59- return None
60-
61- #if butname argument is set, the "name" attributes is removed
62- #from the list
63- def get_all_attributes(self,butname=False) :
64- l = self.attributes.keys()
65- if butname :
66- #Normally this condition is not necessary
67- #Defensiveness...
68- if "name" in l :
69- l.remove("name")
70- return l
71-
72+
73+ def set_attribute(self, att_name, att_value):
74+ """Set an arbitrary attribute.
75+
76+ This will call the 'save_cllbk' callback passed to the constructor.
77+
78+ :param att_name: The name of the attribute.
79+ :param att_value: The value of the attribute. Will be converted to a
80+ string.
81+ """
82+ if att_name == "name":
83+ # Warning : only the constructor can set the "name".
84+ #
85+ # XXX: This should actually raise an exception, or warn, or
86+ # something. The Zen of Python says "Errors should never pass
87+ # silently." -- jml, 2009-07-17
88+ return
89+ # Attributes should all be strings.
90+ val = unicode(str(att_value), "UTF-8")
91+ self._attributes[att_name] = val
92+ self._save()
93+
94+ def get_attribute(self, att_name):
95+ """Get the attribute 'att_name'.
96+
97+ Returns None if there is no attribute matching 'att_name'.
98+ """
99+ return self._attributes.get(att_name, None)
100+
101+ def get_all_attributes(self, butname=False):
102+ """Return a list of all attribute names.
103+
104+ :param butname: If True, exclude 'name' from the list of attribute
105+ names.
106+ """
107+ attributes = self._attributes.keys()
108+ if butname:
109+ attributes.remove('name')
110+ return attributes
111+
112 def __str__(self):
113- return "Tag: %s" %self.get_name()
114-
115+ return "Tag: %s" % self.get_name()
116
117=== modified file 'GTG/tests/__init__.py'
118--- GTG/tests/__init__.py 2009-07-14 12:09:55 +0000
119+++ GTG/tests/__init__.py 2009-07-17 08:35:00 +0000
120@@ -21,10 +21,14 @@
121
122 import unittest
123
124-from GTG.tests import test_backends
125+from GTG.tests import (
126+ test_backends,
127+ test_tagstore,
128+ )
129
130
131 def test_suite():
132 return unittest.TestSuite([
133 test_backends.test_suite(),
134+ test_tagstore.test_suite(),
135 ])
136
137=== added file 'GTG/tests/test_tagstore.py'
138--- GTG/tests/test_tagstore.py 1970-01-01 00:00:00 +0000
139+++ GTG/tests/test_tagstore.py 2009-07-17 10:24:08 +0000
140@@ -0,0 +1,122 @@
141+# -*- coding: utf-8 -*-
142+# -----------------------------------------------------------------------------
143+# Gettings Things Gnome! - a personnal organizer for the GNOME desktop
144+# Copyright (c) 2008-2009 - Lionel Dricot & Bertrand Rousseau
145+#
146+# This program is free software: you can redistribute it and/or modify it under
147+# the terms of the GNU General Public License as published by the Free Software
148+# Foundation, either version 3 of the License, or (at your option) any later
149+# version.
150+#
151+# This program is distributed in the hope that it will be useful, but WITHOUT
152+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
153+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
154+# details.
155+#
156+# You should have received a copy of the GNU General Public License along with
157+# this program. If not, see <http://www.gnu.org/licenses/>.
158+# -----------------------------------------------------------------------------
159+
160+"""Tests for the tagstore."""
161+
162+import unittest
163+
164+from GTG.core.tagstore import Tag
165+
166+
167+class TestTag(unittest.TestCase):
168+ """Tests for `Tag`."""
169+
170+ def test_name(self):
171+ # The first argument to the Tag constructor is the name, which you can
172+ # get with get_name().
173+ tag = Tag('foo')
174+ self.assertEqual('foo', tag.get_name())
175+
176+ def test_name_is_attribute(self):
177+ # The name of the tag is also stored as an attribute.
178+ tag = Tag('foo')
179+ self.assertEqual('foo', tag.get_attribute('name'))
180+
181+ def test_missing_attribute_returns_none(self):
182+ # If get_attribute is called for an attribute that doesn't exist, it
183+ # returns None.
184+ tag = Tag('whatever')
185+ result = tag.get_attribute('no-such-attribute')
186+ self.assertEqual(None, result)
187+
188+ def test_set_attribute_calls_save(self):
189+ # Calling set_attribute calls the 'save_cllbk'.
190+ save_calls = []
191+ tag = Tag('whatever', lambda: save_calls.append(None))
192+ tag.set_attribute('new-attribute', 'value')
193+ self.assertEqual(1, len(save_calls))
194+
195+ def test_set_then_get_attribute(self):
196+ # Attributes set with set_attribute can be retrieved with
197+ # get_attribute.
198+ tag = Tag('whatever', lambda: None)
199+ tag.set_attribute('new-attribute', 'value')
200+ result = tag.get_attribute('new-attribute')
201+ self.assertEqual('value', result)
202+
203+ def test_set_non_str_attribute_casts_to_string(self):
204+ # If the value of the attribute passed to set_attribute is not a
205+ # string, it's cast to a string.
206+ tag = Tag('whatever', lambda: None)
207+ tag.set_attribute('new-attribute', 42)
208+ result = tag.get_attribute('new-attribute')
209+ self.assertEqual('42', result)
210+
211+ def test_get_all_attributes_initial(self):
212+ # Initially, a Tag has only the name attribute.
213+ tag = Tag('foo')
214+ self.assertEqual(['name'], tag.get_all_attributes())
215+
216+ def test_get_all_attributes_after_setting(self):
217+ # After attributes are set, get_all_attributes includes those
218+ # attributes. The order is not guaranteed.
219+ tag = Tag('foo', lambda: None)
220+ tag.set_attribute('bar', 'baz')
221+ self.assertEqual(set(['name', 'bar']), set(tag.get_all_attributes()))
222+
223+ def test_get_all_but_name(self):
224+ # If 'butname' is True, then exclude the 'name' attribute.
225+ tag = Tag('foo', lambda: None)
226+ self.assertEqual([], tag.get_all_attributes(butname=True))
227+ tag.set_attribute('bar', 'baz')
228+ self.assertEqual(['bar'], tag.get_all_attributes(butname=True))
229+
230+ def test_str(self):
231+ # str(tag) is 'Tag: <name>'
232+ tag = Tag('foo')
233+ self.assertEqual('Tag: foo', str(tag))
234+
235+ def test_set_name_attribute_does_nothing(self):
236+ # The 'name' attribute is set by the constructor. After it is set, it
237+ # cannot be changed with further calls to set_attribute.
238+ tag = Tag('old', lambda: None)
239+ tag.set_attribute('name', 'new')
240+ self.assertEqual('old', tag.get_name())
241+ self.assertEqual('old', tag.get_attribute('name'))
242+
243+ # XXX: The following tests check the current behaviour of the Tag class,
244+ # but I'm not sure if they're correct behaviour. -- jml, 2009-07-17
245+
246+ def test_save_not_called_on_construction(self):
247+ # The save callback isn't called by the constructor, despite the fact
248+ # that it sets the name attribute.
249+ save_calls = []
250+ Tag('old', lambda: save_calls.append(None))
251+ self.assertEqual(0, len(save_calls))
252+
253+ def test_set_name_doesnt_call_save(self):
254+ # Setting the name attribute doesn't call save.
255+ save_calls = []
256+ tag = Tag('old', lambda: save_calls.append(None))
257+ tag.set_attribute('name', 'new')
258+ self.assertEqual(0, len(save_calls))
259+
260+
261+def test_suite():
262+ return unittest.TestLoader().loadTestsFromName(__name__)

Subscribers

People subscribed via source and target branches

to status/vote changes: