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

Proposed by Matt Fischer
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
accomplishments/daemon/api.py (+38/-20)
accomplishments/daemon/tests/tests.py (+13/-29)
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 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

minor fix to the error message

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'accomplishments/daemon/api.py'
--- accomplishments/daemon/api.py 2012-07-23 16:27:15 +0000
+++ accomplishments/daemon/api.py 2012-07-30 17:26:22 +0000
@@ -938,24 +938,24 @@
938 if collection in self.accDB:938 if collection in self.accDB:
939 # This collection has already been loaded from another install path!939 # This collection has already been loaded from another install path!
940 continue940 continue
941 941
942 collpath = os.path.join(path,collection)942 collpath = os.path.join(path,collection)
943 aboutpath = os.path.join(collpath,'ABOUT')943 aboutpath = os.path.join(collpath,'ABOUT')
944 944
945 # Load data from ABOUT file945 # Load data from ABOUT file
946 cfg = ConfigParser.RawConfigParser()946 cfg = ConfigParser.RawConfigParser()
947 cfg.read(aboutpath)947 cfg.read(aboutpath)
948 948
949 if not (cfg.has_option("general","langdefault") and cfg.has_option("general","name")):949 if not (cfg.has_option("general","langdefault") and cfg.has_option("general","name")):
950 print aboutpath950 print aboutpath
951 raise LookupError("Accomplishment collection with invalid ABOUT file ")951 raise LookupError("Accomplishment collection with invalid ABOUT file ")
952 952
953 langdefault = cfg.get("general","langdefault")953 langdefault = cfg.get("general","langdefault")
954 collectionname = cfg.get("general","name")954 collectionname = cfg.get("general","name")
955 955
956 collauthors = set()956 collauthors = set()
957 collcategories = {}957 collcategories = {}
958 958
959 langdefaultpath = os.path.join(collpath,langdefault)959 langdefaultpath = os.path.join(collpath,langdefault)
960 setsslist = os.listdir(langdefaultpath)960 setsslist = os.listdir(langdefaultpath)
961 accno = 0961 accno = 0
@@ -968,18 +968,27 @@
968 translatedpath = os.path.join(os.path.join(collpath,self.lang),accomset)968 translatedpath = os.path.join(os.path.join(collpath,self.lang),accomset)
969 if os.path.exists(translatedpath):969 if os.path.exists(translatedpath):
970 # yes, so use the translated file970 # yes, so use the translated file
971 accomcfg.read(translatedpath)971 readpath = translatedpath
972 langused = self.lang972 langused = self.lang
973 else:973 else:
974 # no. maybe there is a shorter language code?974 # no. maybe there is a shorter language code?
975 translatedpath = os.path.join(os.path.join(collpath,self.lang.split("_")[0]),accomset)975 translatedpath = os.path.join(os.path.join(collpath,self.lang.split("_")[0]),accomset)
976 if os.path.exists(translatedpath):976 if os.path.exists(translatedpath):
977 accomcfg.read(translatedpath)977 readpath = translatedpath
978 langused = self.lang.split("_")[0]978 langused = self.lang.split("_")[0]
979 else:979 else:
980 # no. fallback to default one980 # no. fallback to default one
981 accomcfg.read(accompath)981 readpath = accompath
982 langused = langdefault982 langused = langdefault
983
984 # do the parse
985 try:
986 accomcfg.read(readpath)
987 except ConfigParser.ParsingError, e:
988 log.msg("Parse error for %s. Skipping."\
989 "Parse error is: %s" % (readpath, e.message))
990 continue
991
983 accomdata = dict(accomcfg._sections["accomplishment"])992 accomdata = dict(accomcfg._sections["accomplishment"])
984 accomID = collection + "/" + accomset[:-15]993 accomID = collection + "/" + accomset[:-15]
985 if 'author' in accomdata:994 if 'author' in accomdata:
@@ -1026,18 +1035,27 @@
1026 translatedpath = os.path.join(os.path.join(collpath,self.lang),os.path.join(accomset,accomfile))1035 translatedpath = os.path.join(os.path.join(collpath,self.lang),os.path.join(accomset,accomfile))
1027 if os.path.exists(translatedpath):1036 if os.path.exists(translatedpath):
1028 # yes, so use the translated file1037 # yes, so use the translated file
1029 accomcfg.read(translatedpath)1038 readpath = translatedpath
1030 langused = self.lang1039 langused = self.lang
1031 else:1040 else:
1032 # no. maybe there is a shorter language code?1041 # no. maybe there is a shorter language code?
1033 translatedpath = os.path.join(os.path.join(collpath,self.lang.split("_")[0]),os.path.join(accomset,accomfile))1042 translatedpath = os.path.join(os.path.join(collpath,self.lang.split("_")[0]),os.path.join(accomset,accomfile))
1034 if os.path.exists(translatedpath):1043 if os.path.exists(translatedpath):
1035 accomcfg.read(translatedpath)1044 readpath = translatedpath
1036 langused = self.lang.split("_")[0]1045 langused = self.lang.split("_")[0]
1037 else:1046 else:
1038 # no. fallback to default one1047 # no. fallback to default one
1039 accomcfg.read(accompath)1048 readpath = accompath
1040 langused = langdefault1049 langused = langdefault
1050
1051 # do the parse
1052 try:
1053 accomcfg.read(readpath)
1054 except ConfigParser.ParsingError, e:
1055 log.msg("Parse error for %s. Skipping."\
1056 "Parse error is: %s" % (readpath,e.message))
1057 continue
1058
1041 accomdata = dict(accomcfg._sections["accomplishment"])1059 accomdata = dict(accomcfg._sections["accomplishment"])
1042 accomID = collection + "/" + accomfile[:-15]1060 accomID = collection + "/" + accomfile[:-15]
1043 if 'author' in accomdata:1061 if 'author' in accomdata:
@@ -1069,7 +1087,7 @@
1069 accomdata['categories'] = []1087 accomdata['categories'] = []
1070 self.accDB[accomID] = accomdata1088 self.accDB[accomID] = accomdata
1071 accno = accno + 11089 accno = accno + 1
1072 1090
1073 # Look for extrainformation dir1091 # Look for extrainformation dir
1074 extrainfodir = os.path.join(collpath,"extrainformation")1092 extrainfodir = os.path.join(collpath,"extrainformation")
1075 extrainfolist = os.listdir(extrainfodir)1093 extrainfolist = os.listdir(extrainfodir)
@@ -1092,7 +1110,7 @@
1092 description = eicfg.get("description",self.lang.split("_")[0])1110 description = eicfg.get("description",self.lang.split("_")[0])
1093 else:1111 else:
1094 description = eicfg.get("description",langdefault)1112 description = eicfg.get("description",langdefault)
1095 1113
1096 if eicfg.has_option("example", self.lang):1114 if eicfg.has_option("example", self.lang):
1097 example = eicfg.get("example", self.lang)1115 example = eicfg.get("example", self.lang)
1098 elif eicfg.has_option("example", self.lang.split("_")[0]):1116 elif eicfg.has_option("example", self.lang.split("_")[0]):
@@ -1101,29 +1119,29 @@
1101 example = eicfg.get("example", langdefault)1119 example = eicfg.get("example", langdefault)
1102 else:1120 else:
1103 example = None1121 example = None
1104 1122
1105 if eicfg.has_option("regex", "value"):1123 if eicfg.has_option("regex", "value"):
1106 regex = eicfg.get("regex", "value")1124 regex = eicfg.get("regex", "value")
1107 else:1125 else:
1108 regex = None1126 regex = None
1109 1127
1110 extrainfo[extrainfofile] = {1128 extrainfo[extrainfofile] = {
1111 'label': label,1129 'label': label,
1112 'description': description,1130 'description': description,
1113 'example': example,1131 'example': example,
1114 'regex': regex,1132 'regex': regex,
1115 }1133 }
1116 1134
1117 # Store data about this colection1135 # Store data about this colection
1118 collectiondata = {'langdefault':langdefault,'name':collectionname, 'acc_num':accno, 'type':"collection", 'base-path': collpath, 'categories' : collcategories, 'extra-information': extrainfo, 'authors':collauthors}1136 collectiondata = {'langdefault':langdefault,'name':collectionname, 'acc_num':accno, 'type':"collection", 'base-path': collpath, 'categories' : collcategories, 'extra-information': extrainfo, 'authors':collauthors}
1119 self.accDB[collection] = collectiondata1137 self.accDB[collection] = collectiondata
1120 1138
1121 self._update_all_locked_and_completed_statuses()1139 self._update_all_locked_and_completed_statuses()
1122 # Uncomment following for debugging1140 # Uncomment following for debugging
1123 # print self.accDB\1141 # print self.accDB\
1124 1142
1125 # ======= Access functions =======1143 # ======= Access functions =======
1126 1144
1127 def get_acc_data(self,accomID):1145 def get_acc_data(self,accomID):
1128 return self.accDB[accomID]1146 return self.accDB[accomID]
1129 1147
11301148
=== modified file 'accomplishments/daemon/tests/tests.py'
--- accomplishments/daemon/tests/tests.py 2012-07-19 19:47:52 +0000
+++ accomplishments/daemon/tests/tests.py 2012-07-30 17:26:22 +0000
@@ -137,37 +137,33 @@
137 shutil.rmtree(self.td)137 shutil.rmtree(self.td)
138138
139 def test_run_scripts(self):139 def test_run_scripts(self):
140 # due to LP1030208, if the daemon is running (like on a dev box)
141 # it will pop off the test, but in a pbuilder or build system
142 # there will be no daemon, so we can just ensure that this
143 # doesn't crash
140 self.util_copy_accomp(self.accomp_dir, "third")144 self.util_copy_accomp(self.accomp_dir, "third")
141 a = api.Accomplishments(None, None, True)145 a = api.Accomplishments(None, None, True)
142146
143 # pass in None147 # pass in None
144 self.assertEqual(a.run_scripts(None), None)148 self.assertEqual(a.run_scripts(None), None)
145 self.assertEqual(a.scripts_queue, deque(["%s/third" % self.ACCOMP_SET]))
146 self.assertEqual(a.scripts_queue.popleft(),
147 "%s/third" % self.ACCOMP_SET)
148149
149 # pass in a bad arg150 # pass in a bad arg
150 self.assertEqual(a.run_scripts(122), None)151 self.assertEqual(a.run_scripts(122), None)
151 self.assertEqual(a.scripts_queue, deque(["%s/third" % self.ACCOMP_SET]))
152 self.assertEqual(a.scripts_queue.popleft(),
153 "%s/third" % self.ACCOMP_SET)
154152
155 # pass in a specific item153 # pass in a specific item
156 self.assertEqual(a.run_scripts(["%s/third" % self.ACCOMP_SET]), None)154 self.assertEqual(a.run_scripts(["%s/third" % self.ACCOMP_SET]), None)
157 self.assertEqual(a.scripts_queue, deque(["%s/third" % self.ACCOMP_SET]))
158 self.assertEqual(a.scripts_queue.popleft(),
159 "%s/third" % self.ACCOMP_SET)
160155
161 def test_run_script(self):156 def test_run_script(self):
157 # due to LP1030208, if the daemon is running (like on a dev box)
158 # it will pop off the test, but in a pbuilder or build system
159 # there will be no daemon, so we can just ensure that this
160 # doesn't crash
162 self.util_copy_accomp(self.accomp_dir, "third")161 self.util_copy_accomp(self.accomp_dir, "third")
163 a = api.Accomplishments(None, None, True)162 a = api.Accomplishments(None, None, True)
164 self.assertEqual(a.run_script("%s/wrong" % self.ACCOMP_SET), None)163 self.assertEqual(a.run_script("%s/wrong" % self.ACCOMP_SET), None)
165 self.assertEqual(a.scripts_queue, deque([]))
166 self.assertEqual(a.run_script("wrong"), None)164 self.assertEqual(a.run_script("wrong"), None)
167 self.assertEqual(a.scripts_queue, deque([]))
168165
169 self.assertEqual(a.run_script("%s/third" % self.ACCOMP_SET), None)166 self.assertEqual(a.run_script("%s/third" % self.ACCOMP_SET), None)
170 self.assertEqual(a.scripts_queue, deque(["%s/third" % self.ACCOMP_SET]))
171167
172 def test_get_acc_date_completed(self):168 def test_get_acc_date_completed(self):
173 self.util_remove_all_accomps(self.accomp_dir)169 self.util_remove_all_accomps(self.accomp_dir)
@@ -524,7 +520,6 @@
524 # put the file back520 # put the file back
525 self.util_write_about_file(self.accomp_root)521 self.util_write_about_file(self.accomp_root)
526522
527 @unittest.skip("waiting for LP:1024041 to be fixed")
528 def test_bad_accomplishment_list(self):523 def test_bad_accomplishment_list(self):
529 # this test ensures that a bad accompishment doesn't crash the524 # this test ensures that a bad accompishment doesn't crash the
530 # daemon or get into the list525 # daemon or get into the list
@@ -540,25 +535,14 @@
540 a.reload_accom_database()535 a.reload_accom_database()
541 self.assertEqual(len(a.list_accomplishments()), 1)536 self.assertEqual(len(a.list_accomplishments()), 1)
542537
538 self.util_write_file(self.accomp_dir, "bad.accomplishment",
539 "descriptionbad desc\n")
540 a.reload_accom_database()
541 self.assertEqual(len(a.list_accomplishments()), 1)
542
543 # cleanup543 # cleanup
544 self.util_remove_all_accomps(self.accomp_dir)544 self.util_remove_all_accomps(self.accomp_dir)
545545
546 def test_bad_accomplishment_parse(self):
547 self.util_write_file(self.accomp_dir, "bad.accomplishment",
548 "[accomplishment]\n"\
549 "descriptionbad desc\n")
550 self.assertRaises(ConfigParser.ParsingError, api.Accomplishments, None,
551 None, True)
552 os.remove(os.path.join(self.accomp_dir, "bad.accomplishment"))
553
554 self.util_write_file(self.accomp_dir, "bad.accomplishment",
555 "[accomplishment]\n"\
556 "titlewhatever\n"\
557 "description=bad desc\n")
558 self.assertRaises(ConfigParser.ParsingError, api.Accomplishments, None,
559 None, True)
560 os.remove(os.path.join(self.accomp_dir, "bad.accomplishment"))
561
562 # also tests get_config_value()546 # also tests get_config_value()
563 def test_write_config_file_item(self):547 def test_write_config_file_item(self):
564 a = api.Accomplishments(None, None, True)548 a = api.Accomplishments(None, None, True)

Subscribers

People subscribed via source and target branches