Merge lp:~thekorn/zeitgeist/fix-595411-remote_testcases into lp:zeitgeist/0.1

[Markus Korn]

- it always restores the values of os.environ if changed to zeitgeist specific values
- make sure to call the test methods (self.assertEquals() etc.) in the main process, and not in a callback, this exposes failures in the test report
- added RemoteTestCase.{deleteEventsAndWait,findRelatedEventsAndWait} in testutils.py
- small API CHANGE of the experimental DeleteEvents()-method: this method now returns a time-tuple describing the intervall where events were changed, otherwise there won't be any direct way to test this method (without using a monitor)

This branch fixes a few issues in our testsuite:
- it always restores the values of os.environ if changed to zeitgeist specific values
- make sure to call the test methods (self.assertEquals() etc.) in the main process, and not in a callback, this exposes failures in the test report
- added RemoteTestCase.{deleteEventsAndWait,findRelatedEventsAndWait} in testutils.py
- small API CHANGE of the experimental DeleteEvents()-method: this method now returns a time-tuple describing the intervall where events were changed, otherwise there won't be any direct way to test this method (without using a monitor).

[Seif Lotfy (seif)]

Code seems clean, some nifty test cases and it works smoothly
Good Job

[Seif Lotfy (seif)]

Seif Lotfy wrote 24 seconds ago
Code seems clean, some nifty test cases and it works smoothly
Good Job

review: Approve

[Markus Korn (thekorn)]

ok, merged into lp:zeitgeist as rev 1522, thanks seif for the review

[Siegfried Gevatter (rainct)]

Good work fixing the tests!

As I said on IRC, I disagree with this chunk:
> small API CHANGE of the experimental DeleteEvents()-method: this method
> now returns a time-tuple describing the intervall where events were

Have you thought about it?

review: Needs Fixing

=== modified file '_zeitgeist/engine/remote.py'
--- _zeitgeist/engine/remote.py	2010-07-01 07:34:26 +0000
+++ _zeitgeist/engine/remote.py	2010-07-22 09:54:43 +0000
@@ -308,7 +308,7 @@
 	
 	@dbus.service.method(constants.DBUS_INTERFACE,
 	                     in_signature="au",
-	                     out_signature="",
+	                     out_signature="(ii)",
 	                     sender_keyword="sender")
 	def DeleteEvents(self, event_ids, sender):
 		"""Delete a set of events from the log given their IDs
@@ -324,6 +324,13 @@
 			# have been deleted before or the IDs might even have been invalid.
 			self._notifications.notify_delete(
 			    TimeRange(timestamps[0], timestamps[1]), event_ids)
+		if timestamps is None:
+			# unknown event id, see doc of delete_events()
+			return (-1, -1)
+		timestamp_start, timestamp_end = timestamps
+		timestamp_start = timestamp_start if timestamp_start is not None else -1
+		timestamp_end = timestamp_end if timestamp_end is not None else -1
+		return (timestamp_start, timestamp_end)
 
 	@dbus.service.method(constants.DBUS_INTERFACE, in_signature="", out_signature="")
 	def DeleteLog(self):

=== modified file 'test/engine-extension-test.py'
--- test/engine-extension-test.py	2010-07-01 07:38:58 +0000
+++ test/engine-extension-test.py	2010-07-22 09:54:43 +0000
@@ -32,10 +32,21 @@
 	
 	def setUp (self):
 		constants.DATABASE_FILE = ":memory:"
-		constants.DEFAULT_EXTENSIONS = []
+		self.save_default_ext = os.environ.get("ZEITGEIST_DEFAULT_EXTENSIONS")
+		self.save_extra_ext = os.environ.get("ZEITGEIST_EXTRA_EXTENSIONS")
+		os.environ["ZEITGEIST_DEFAULT_EXTENSIONS"] = ""
+		os.environ["ZEITGEIST_EXTRA_EXTENSIONS"] = ""
 		self.engine = get_engine()
 		
 	def tearDown (self):
+		if self.save_default_ext is not None:
+			os.environ["ZEITGEIST_DEFAULT_EXTENSIONS"] = self.save_default_ext
+		else:
+			del os.environ["ZEITGEIST_DEFAULT_EXTENSIONS"]
+		if self.save_extra_ext is not None:
+			os.environ["ZEITGEIST_EXTRA_EXTENSIONS"] = self.save_extra_ext
+		else:
+			del os.environ["ZEITGEIST_EXTRA_EXTENSIONS"]
 		self.engine.close()
 		_zeitgeist.engine._engine = None
 

=== modified file 'test/engine-test.py'
--- test/engine-test.py	2010-07-01 08:04:55 +0000
+++ test/engine-test.py	2010-07-22 09:54:43 +0000
@@ -40,6 +40,8 @@
 class _engineTestClass(unittest.TestCase):
 	
 	def setUp (self):
+		self.save_default_ext = os.environ.get("ZEITGEIST_DEFAULT_EXTENSIONS")
+		self.save_extra_ext = os.environ.get("ZEITGEIST_EXTRA_EXTENSIONS")
 		os.environ["ZEITGEIST_DEFAULT_EXTENSIONS"] = ""
 		os.environ["ZEITGEIST_EXTRA_EXTENSIONS"] = ""
 		global test_event_1
@@ -55,6 +57,14 @@
 		self.engine = get_engine()
 	
 	def tearDown (self):
+		if self.save_default_ext is not None:
+			os.environ["ZEITGEIST_DEFAULT_EXTENSIONS"] = self.save_default_ext
+		else:
+			del os.environ["ZEITGEIST_DEFAULT_EXTENSIONS"]
+		if self.save_extra_ext is not None:
+			os.environ["ZEITGEIST_EXTRA_EXTENSIONS"] = self.save_extra_ext
+		else:
+			del os.environ["ZEITGEIST_EXTRA_EXTENSIONS"]
 		self.engine.close()
 		_zeitgeist.engine._engine = None
 

=== modified file 'test/remote-test.py'
--- test/remote-test.py	2010-05-15 13:05:14 +0000
+++ test/remote-test.py	2010-07-22 09:54:43 +0000
@@ -252,27 +252,28 @@
 		self.assertEquals(1, result.pop())
 		
 	def testDeleteEvents(self):
-		result = []
-		mainloop = gobject.MainLoop()
+		""" delete all events with actor == firefox """
 		events = parse_events("test/data/five_events.js")
-		self.client.insert_events(events)
+		self.insertEventsAndWait(events)
 		
 		event = Event()
 		event.actor = "firefox"
 		
-		ids = self.findEventIdsAndWait([event])
-		
-		def callback():	
-			ids = self.findEventIdsAndWait([])
-			self.assertEquals(2, len(ids))
-			
-		self.client.delete_events(ids, callback)
+		# get event ids with actor == firefox
+		ff_ids = self.findEventIdsAndWait([event])
+		# delete this events
+		time_range = self.deleteEventsAndWait(ff_ids)
+		# got timerange of deleted events
+		self.assertEquals(2, len(time_range))
+		# get all events, the one with actor == firefox should
+		# not be there
+		ids = self.findEventIdsAndWait([])
+		self.assertEquals(2, len(ids))
+		self.assertEquals(0, len(set(ff_ids) & set(ids)))
 		
 	def testFindByRandomActorAndGet(self):
-		result = []
-		mainloop = gobject.MainLoop()
 		events = parse_events("test/data/five_events.js")
-		self.client.insert_events(events)
+		self.insertEventsAndWait(events)
 		
 		template = Event.new_for_values(actor="/usr/bliblablu")
 		
@@ -283,35 +284,34 @@
 		self.assertEquals(len(events), 0)
 	
 	def testFindRelated(self):
-		mainloop = gobject.MainLoop()
 		events = parse_events("test/data/apriori_events.js")
-		self.client.insert_events(events)
-		
-		def callback(uris):
-			mainloop.quit()
-			self.assertEquals(uris, ["i2", "i1", "i3", "i5"])
-		
-		result = self.client.find_related_uris_for_uris(["i4"], callback, num_events=4, result_type=0)
-		mainloop.run()
+		self.insertEventsAndWait(events)
+		
+		uris = self.findRelatedAndWait(["i4"], num_events=4, result_type=0)
+		self.assertEquals(set(uris), set(["i2", "i1", "i3", "i5"]))
 		
 	def testFindEventsForValues(self):
-		mainloop = gobject.MainLoop()
+		mainloop = gobject.MainLoop() # we don't have an *AndWait-helper method
+									  # for the method we would like to test,
+									  # this is why we need our own local loop
 		events = parse_events("test/data/apriori_events.js")
-		self.client.insert_events(events)
+		self.insertEventsAndWait(events)
 		
+		result = []
 		def callback(events):
+			result.extend(events)
 			mainloop.quit()
-			self.assertEquals(len(events), 1)
-			self.assertEquals(events[0].actor, "firefox")
 		
-		result = self.client.find_events_for_values(callback, actor="firefox", num_events=1)
+		self.client.find_events_for_values(callback, actor="firefox", num_events=1)
 		mainloop.run()
+		self.assertEquals(len(result), 1)
+		self.assertEquals(result[0].actor, "firefox")
 
 	def testDataSourcesRegistry(self):
 		""" Ensure that the DataSourceRegistry extension is there. If we'd want
 		    to do any actual value checking we need to change testutils.py to
 		    use a ZEITGEIST_DATA_PATH other than ~/.local/share. """
-		iface = ZeitgeistDBusInterface()
+		iface = self.client._iface # we know that client._iface is as clean as possible
 		registry = iface.get_extension("DataSourceRegistry", "data_source_registry")
 		registry.GetDataSources()
 	

=== modified file 'test/testutils.py'
--- test/testutils.py	2010-05-15 13:05:14 +0000
+++ test/testutils.py	2010-07-22 09:54:43 +0000
@@ -181,3 +181,42 @@
 		self.client.get_events(event_ids, collect_events_and_quit)
 		mainloop.run()
 		return result
+		
+	def deleteEventsAndWait(self, event_ids):
+		"""
+		Delete events given by their id and run a loop until the result 
+		containing a timetuple describing the interval of changes is
+		returned.
+		
+		This method is basically just a hack to invoke an async method
+		in a blocking manner.
+		"""
+		mainloop = gobject.MainLoop()
+		result = []
+		
+		def collect_timestamp_and_quit(timestamps):
+			result.append(timestamps)
+			mainloop.quit()
+		
+		self.client.delete_events(event_ids, collect_timestamp_and_quit)
+		mainloop.run()
+		return result[0]
+		
+	def findRelatedAndWait(self, subject_uris, num_events, result_type):
+		"""
+		Find related subject uris to given uris and return them.
+		
+		This method is basically just a hack to invoke an async method
+		in a blocking manner.
+		"""
+		mainloop = gobject.MainLoop()
+		result = []
+		
+		def callback(uri_list):
+			result.extend(uri_list)
+			mainloop.quit()
+		
+		self.client.find_related_uris_for_uris(subject_uris, callback,
+			num_events=num_events, result_type=result_type)
+		mainloop.run()
+		return result