Merge lp:~robru/python-dbusmock/querycalls into lp:~pitti/python-dbusmock/old-bzr-trunk

[Robert Bruce Park]

This change makes it possible to query for the called method signatures over dbus, and also makes it possible to clear this log, such that each individual test can start with a fresh state (so no worries about one test checking the log and finding method invocations logged from the previous test).

Let me know how you feel about the implementation, I would be happy to alter it to fit your standards.

[lp:~robru/python-dbusmock/querycalls]

101. By Robert Bruce Park on 2013-01-14

Use str.index instead of str.find.

The reason for this is that str.index will raise an exception if the
substring is not found, which will cause the test to fail; str.find
will return -1 if the substring is not found, which technically
evaluates to true, and thus doesn't cause the test to fail at all if
the string is not found.

[Martin Pitt (pitti)]

I think this is a great idea, thanks for working on this!

If a program uses the D-BUS queries for the logs, and it wants to suppress stdout, you can already use "-l /dev/null", so I don't think we need another command line argument for this. Do you agree?

My main nitpick with this is QueryCalls() returning a single string, which the caller then has to pick apart again. In some cases you just want to know "has this function being called", but in other cases you actually want to verify the arguments of a particular call. So how about:

 - ClearLog() sounds good to me.

 - GetLog() ("Query.." is unusual in D-BUS interfaces) has a return signature a(tsav), returning a list of (timestamp, method, args) tuples.

 - GetMethodCalls(method) has a return signature a(tav), returning a list of (timestamp, args) tuples; by merely checking the returned list length against 0, a caller can conveniently check whether a method was called.

> self.assertTrue(self.dbus_mock.QueryCalls().index(' Wop "foo"'))

Please don't use this test form, it looks confusing as index() can return 0 if the string is right at the start, and 0 counts as False. Please use

  self.assertTrue('search' in QueryCalls() ...)

(Won't be relevant after above API changes, of course).

+ def test_new_logging(self):

Please name this "test_dbus_logging".

Thanks!

review: Needs Fixing

[Martin Pitt (pitti)]

> - GetLog()

Perhaps we should call this GetCalls() for consistency? As "Log" might also indicate information about the mock process itself, which we are not interested in here.

[Robert Bruce Park (robru)]

Sounds good, Martin. I am just running around moving into my new apartment, but I will find time this evening to work on this. So you'll be able to merge it tomorrow ;-)

Martin Pitt <email address hidden> wrote:

>> - GetLog()
>
>Perhaps we should call this GetCalls() for consistency? As "Log" might also indicate information about the mock process itself, which we are not interested in here.
>--
>https://code.launchpad.net/~robru/python-dbusmock/querycalls/+merge/143153
>You are the owner of lp:~robru/python-dbusmock/querycalls.

[lp:~robru/python-dbusmock/querycalls]

102. By Robert Bruce Park on 2013-01-16

Cleanup API as per Martin Pitt's requests.

[Robert Bruce Park (robru)]

Ok pitti, I think you should find this to your liking now. I even expanded the test coverage a little ;-)

review: Approve

[Martin Pitt (pitti)]

This looks good, thanks! I cleaned up the test cases a bit, and added some documentation/docstrings:

  http://bazaar.launchpad.net/~pitti/python-dbusmock/trunk/revision/100

review: Approve

=== modified file 'dbusmock/mockobject.py'
--- dbusmock/mockobject.py	2012-12-19 14:54:18 +0000
+++ dbusmock/mockobject.py	2013-01-16 02:34:20 +0000
@@ -69,6 +69,7 @@
             self.logfile = open(logfile, 'w')
         else:
             self.logfile = None
+        self.call_log = []
 
     def __del__(self):
         if self.logfile:
@@ -344,6 +345,27 @@
 
         dbus_fn(self, *args)
 
+    @dbus.service.method(MOCK_IFACE,
+                         in_signature='',
+                         out_signature='a(tsav)')
+    def GetCalls(self):
+        '''List all the logged calls since the last call to ClearCalls.'''
+        return self.call_log
+
+    @dbus.service.method(MOCK_IFACE,
+                         in_signature='s',
+                         out_signature='a(tav)')
+    def GetMethodCalls(self, method):
+        '''List all the logged calls of a particular method.'''
+        return [(row[0], row[2]) for row in self.call_log if row[1] == method]
+
+    @dbus.service.method(MOCK_IFACE,
+                         in_signature='',
+                         out_signature='')
+    def ClearCalls(self):
+        '''Empty the log of mock call signatures'''
+        self.call_log = []
+
     def mock_method(self, interface, dbus_method, in_signature, *args, **kwargs):
         '''Master mock method.
 
@@ -362,6 +384,7 @@
         args = m.get_args_list()
 
         self.log(dbus_method + self.format_args(args))
+        self.call_log.append((int(time.time()), str(dbus_method), args))
 
         code = self.methods[interface][dbus_method][2]
         if code:

=== modified file 'tests/test_api.py'
--- tests/test_api.py	2012-12-19 11:44:13 +0000
+++ tests/test_api.py	2013-01-16 02:34:20 +0000
@@ -414,6 +414,53 @@
         check('i', ['hello'], 'TypeError: an integer is required')
         check('s', [1], 'TypeError: Expected a string')
 
+    def test_dbus_get_log(self):
+        '''Ensure that the logs can be queried over DBus.'''
+
+        self.dbus_mock.AddMethod('', 'Do', '', '', '')
+        self.assertEqual(self.dbus_test.Do(), None)
+        mock_log = self.dbus_mock.GetCalls()
+        self.assertEqual(len(mock_log), 1)
+        self.assertTrue('Do' in mock_log[0])
+        self.assertEqual(self.dbus_mock.ClearCalls(), None)
+        self.assertEqual(self.dbus_mock.GetCalls(), dbus.Array([]))
+
+        self.dbus_mock.AddMethod('', 'Wop', 's', 's', 'ret="hello"')
+        self.assertEqual(self.dbus_test.Wop('foo'), 'hello')
+        self.assertEqual(self.dbus_test.Wop('bar'), 'hello')
+        mock_log = self.dbus_mock.GetCalls()
+        self.assertEqual(len(mock_log), 2)
+        self.assertTrue('Wop' in mock_log[0])
+        self.assertTrue('foo' in mock_log[0][2])
+        self.assertTrue('Wop' in mock_log[1])
+        self.assertTrue('bar' in mock_log[1][2])
+        self.assertEqual(self.dbus_mock.ClearCalls(), None)
+        self.assertEqual(self.dbus_mock.GetCalls(), dbus.Array([]))
+
+    def test_dbus_get_method_calls(self):
+        '''Ensure that the logs can be queried over DBus.'''
+
+        self.dbus_mock.AddMethod('', 'Do', '', '', '')
+        self.assertEqual(self.dbus_test.Do(), None)
+        self.assertEqual(self.dbus_test.Do(), None)
+        mock_calls = self.dbus_mock.GetMethodCalls('Do')
+        self.assertEqual(len(mock_calls), 2)
+        self.assertEqual(mock_calls[0][1], [])
+        self.assertEqual(mock_calls[1][1], [])
+        self.assertEqual(self.dbus_mock.ClearCalls(), None)
+        self.assertEqual(self.dbus_mock.GetMethodCalls('Do'), dbus.Array([]))
+
+        self.dbus_mock.AddMethod('', 'Wop', 's', 's', 'ret="hello"')
+        self.assertEqual(self.dbus_test.Do(), None)
+        self.assertEqual(self.dbus_test.Wop('foo'), 'hello')
+        self.assertEqual(self.dbus_test.Wop('bar'), 'hello')
+        mock_calls = self.dbus_mock.GetMethodCalls('Wop')
+        self.assertEqual(len(mock_calls), 2)
+        self.assertEqual(mock_calls[0][1], ['foo'])
+        self.assertEqual(mock_calls[1][1], ['bar'])
+        self.assertEqual(self.dbus_mock.ClearCalls(), None)
+        self.assertEqual(self.dbus_mock.GetMethodCalls('Wop'), [])
+
 if __name__ == '__main__':
     # avoid writing to stderr
     unittest.main(testRunner=unittest.TextTestRunner(stream=sys.stdout, verbosity=2))