Ubuntu Accomplishments Daemon

Merge lp:~mfisch/ubuntu-accomplishments-daemon/ubuntu-accomplishments-daemon-parse-fix into lp:ubuntu-accomplishments-daemon

Proposed by Matt Fischer on 2012-07-30
Status: Merged
Merged at revision: 117
Proposed branch: lp:~mfisch/ubuntu-accomplishments-daemon/ubuntu-accomplishments-daemon-parse-fix
Merge into: lp:ubuntu-accomplishments-daemon
Diff against target: 239 lines (+51/-49) 2 files modified
To merge this branch: bzr merge lp:~mfisch/ubuntu-accomplishments-daemon/ubuntu-accomplishments-daemon-parse-fix
Reviewer Review Type Date Requested Status
Ubuntu Accomplishments Daemon Developers 2012-07-30 Pending
Review via email: mp+117307@code.launchpad.net

Description of the Change

Fixes the broken unit tests, which behaved differently depending on whether you had a daemon running on your system. (LP: 1030208

Fixes the daemon crash when there is a parse error in the .accomplishment file. I've modified the unit tests to account for this. You can see where I wrapped the Parse exceptions with a try/except block. Ideally I think we could move large chunks of this piece of code to separate functions.

If you test this, please run the unit tests with and without a daemon running on your system.

To post a comment you must log in.
118. By Matt Fischer on 2012-07-30

minor fix to the error message

Preview Diff

1=== modified file 'accomplishments/daemon/api.py'
2--- accomplishments/daemon/api.py 2012-07-23 16:27:15 +0000
3+++ accomplishments/daemon/api.py 2012-07-30 17:26:22 +0000
4@@ -938,24 +938,24 @@
5 if collection in self.accDB:
6 # This collection has already been loaded from another install path!
7 continue
8-
9+
10 collpath = os.path.join(path,collection)
11 aboutpath = os.path.join(collpath,'ABOUT')
12-
13+
14 # Load data from ABOUT file
15 cfg = ConfigParser.RawConfigParser()
16 cfg.read(aboutpath)
17-
18+
19 if not (cfg.has_option("general","langdefault") and cfg.has_option("general","name")):
20 print aboutpath
21 raise LookupError("Accomplishment collection with invalid ABOUT file ")
22-
23+
24 langdefault = cfg.get("general","langdefault")
25 collectionname = cfg.get("general","name")
26-
27+
28 collauthors = set()
29 collcategories = {}
30-
31+
32 langdefaultpath = os.path.join(collpath,langdefault)
33 setsslist = os.listdir(langdefaultpath)
34 accno = 0
35@@ -968,18 +968,27 @@
36 translatedpath = os.path.join(os.path.join(collpath,self.lang),accomset)
37 if os.path.exists(translatedpath):
38 # yes, so use the translated file
39- accomcfg.read(translatedpath)
40+ readpath = translatedpath
41 langused = self.lang
42 else:
43 # no. maybe there is a shorter language code?
44 translatedpath = os.path.join(os.path.join(collpath,self.lang.split("_")[0]),accomset)
45 if os.path.exists(translatedpath):
46- accomcfg.read(translatedpath)
47+ readpath = translatedpath
48 langused = self.lang.split("_")[0]
49 else:
50 # no. fallback to default one
51- accomcfg.read(accompath)
52+ readpath = accompath
53 langused = langdefault
54+
55+ # do the parse
56+ try:
57+ accomcfg.read(readpath)
58+ except ConfigParser.ParsingError, e:
59+ log.msg("Parse error for %s. Skipping."\
60+ "Parse error is: %s" % (readpath, e.message))
61+ continue
62+
63 accomdata = dict(accomcfg._sections["accomplishment"])
64 accomID = collection + "/" + accomset[:-15]
65 if 'author' in accomdata:
66@@ -1026,18 +1035,27 @@
67 translatedpath = os.path.join(os.path.join(collpath,self.lang),os.path.join(accomset,accomfile))
68 if os.path.exists(translatedpath):
69 # yes, so use the translated file
70- accomcfg.read(translatedpath)
71+ readpath = translatedpath
72 langused = self.lang
73 else:
74 # no. maybe there is a shorter language code?
75 translatedpath = os.path.join(os.path.join(collpath,self.lang.split("_")[0]),os.path.join(accomset,accomfile))
76 if os.path.exists(translatedpath):
77- accomcfg.read(translatedpath)
78+ readpath = translatedpath
79 langused = self.lang.split("_")[0]
80 else:
81 # no. fallback to default one
82- accomcfg.read(accompath)
83+ readpath = accompath
84 langused = langdefault
85+
86+ # do the parse
87+ try:
88+ accomcfg.read(readpath)
89+ except ConfigParser.ParsingError, e:
90+ log.msg("Parse error for %s. Skipping."\
91+ "Parse error is: %s" % (readpath,e.message))
92+ continue
93+
94 accomdata = dict(accomcfg._sections["accomplishment"])
95 accomID = collection + "/" + accomfile[:-15]
96 if 'author' in accomdata:
97@@ -1069,7 +1087,7 @@
98 accomdata['categories'] = []
99 self.accDB[accomID] = accomdata
100 accno = accno + 1
101-
102+
103 # Look for extrainformation dir
104 extrainfodir = os.path.join(collpath,"extrainformation")
105 extrainfolist = os.listdir(extrainfodir)
106@@ -1092,7 +1110,7 @@
107 description = eicfg.get("description",self.lang.split("_")[0])
108 else:
109 description = eicfg.get("description",langdefault)
110-
111+
112 if eicfg.has_option("example", self.lang):
113 example = eicfg.get("example", self.lang)
114 elif eicfg.has_option("example", self.lang.split("_")[0]):
115@@ -1101,29 +1119,29 @@
116 example = eicfg.get("example", langdefault)
117 else:
118 example = None
119-
120+
121 if eicfg.has_option("regex", "value"):
122 regex = eicfg.get("regex", "value")
123 else:
124 regex = None
125-
126+
127 extrainfo[extrainfofile] = {
128 'label': label,
129 'description': description,
130 'example': example,
131 'regex': regex,
132 }
133-
134+
135 # Store data about this colection
136 collectiondata = {'langdefault':langdefault,'name':collectionname, 'acc_num':accno, 'type':"collection", 'base-path': collpath, 'categories' : collcategories, 'extra-information': extrainfo, 'authors':collauthors}
137 self.accDB[collection] = collectiondata
138-
139+
140 self._update_all_locked_and_completed_statuses()
141 # Uncomment following for debugging
142 # print self.accDB\
143-
144+
145 # ======= Access functions =======
146-
147+
148 def get_acc_data(self,accomID):
149 return self.accDB[accomID]
150
151
152=== modified file 'accomplishments/daemon/tests/tests.py'
153--- accomplishments/daemon/tests/tests.py 2012-07-19 19:47:52 +0000
154+++ accomplishments/daemon/tests/tests.py 2012-07-30 17:26:22 +0000
155@@ -137,37 +137,33 @@
156 shutil.rmtree(self.td)
157
158 def test_run_scripts(self):
159+ # due to LP1030208, if the daemon is running (like on a dev box)
160+ # it will pop off the test, but in a pbuilder or build system
161+ # there will be no daemon, so we can just ensure that this
162+ # doesn't crash
163 self.util_copy_accomp(self.accomp_dir, "third")
164 a = api.Accomplishments(None, None, True)
165
166 # pass in None
167 self.assertEqual(a.run_scripts(None), None)
168- self.assertEqual(a.scripts_queue, deque(["%s/third" % self.ACCOMP_SET]))
169- self.assertEqual(a.scripts_queue.popleft(),
170- "%s/third" % self.ACCOMP_SET)
171
172 # pass in a bad arg
173 self.assertEqual(a.run_scripts(122), None)
174- self.assertEqual(a.scripts_queue, deque(["%s/third" % self.ACCOMP_SET]))
175- self.assertEqual(a.scripts_queue.popleft(),
176- "%s/third" % self.ACCOMP_SET)
177
178 # pass in a specific item
179 self.assertEqual(a.run_scripts(["%s/third" % self.ACCOMP_SET]), None)
180- self.assertEqual(a.scripts_queue, deque(["%s/third" % self.ACCOMP_SET]))
181- self.assertEqual(a.scripts_queue.popleft(),
182- "%s/third" % self.ACCOMP_SET)
183
184 def test_run_script(self):
185+ # due to LP1030208, if the daemon is running (like on a dev box)
186+ # it will pop off the test, but in a pbuilder or build system
187+ # there will be no daemon, so we can just ensure that this
188+ # doesn't crash
189 self.util_copy_accomp(self.accomp_dir, "third")
190 a = api.Accomplishments(None, None, True)
191 self.assertEqual(a.run_script("%s/wrong" % self.ACCOMP_SET), None)
192- self.assertEqual(a.scripts_queue, deque([]))
193 self.assertEqual(a.run_script("wrong"), None)
194- self.assertEqual(a.scripts_queue, deque([]))
195
196 self.assertEqual(a.run_script("%s/third" % self.ACCOMP_SET), None)
197- self.assertEqual(a.scripts_queue, deque(["%s/third" % self.ACCOMP_SET]))
198
199 def test_get_acc_date_completed(self):
200 self.util_remove_all_accomps(self.accomp_dir)
201@@ -524,7 +520,6 @@
202 # put the file back
203 self.util_write_about_file(self.accomp_root)
204
205- @unittest.skip("waiting for LP:1024041 to be fixed")
206 def test_bad_accomplishment_list(self):
207 # this test ensures that a bad accompishment doesn't crash the
208 # daemon or get into the list
209@@ -540,25 +535,14 @@
210 a.reload_accom_database()
211 self.assertEqual(len(a.list_accomplishments()), 1)
212
213+ self.util_write_file(self.accomp_dir, "bad.accomplishment",
214+ "descriptionbad desc\n")
215+ a.reload_accom_database()
216+ self.assertEqual(len(a.list_accomplishments()), 1)
217+
218 # cleanup
219 self.util_remove_all_accomps(self.accomp_dir)
220
221- def test_bad_accomplishment_parse(self):
222- self.util_write_file(self.accomp_dir, "bad.accomplishment",
223- "[accomplishment]\n"\
224- "descriptionbad desc\n")
225- self.assertRaises(ConfigParser.ParsingError, api.Accomplishments, None,
226- None, True)
227- os.remove(os.path.join(self.accomp_dir, "bad.accomplishment"))
228-
229- self.util_write_file(self.accomp_dir, "bad.accomplishment",
230- "[accomplishment]\n"\
231- "titlewhatever\n"\
232- "description=bad desc\n")
233- self.assertRaises(ConfigParser.ParsingError, api.Accomplishments, None,
234- None, True)
235- os.remove(os.path.join(self.accomp_dir, "bad.accomplishment"))
236-
237 # also tests get_config_value()
238 def test_write_config_file_item(self):
239 a = api.Accomplishments(None, None, True)

Subscribers

People subscribed via source and target branches