Code review comment for lp:~mandel/ubuntuone-client/improve-fs-notifications-logging

> * The following code is not needed since the memento handler accepts multiple
> strings in one single call:
>
> 119 + def _assert_logs(self, events):
> 120 + """Assert the debug logs."""
> 121 + logs = self._get_fs_logs(events)
> 122 + for msg in logs:
> 123 + self.assertTrue(self.memento.check_debug(msg))
>
> you just have to do:
>
> self.assertTrue(self.memento.check_debug(*self._get_fs_logs(events)))
>
> * I also think that all the new helper functions could be improve using list
> comprehensions, for example:
>
> 86 + def _get_event_msgs(self, events):
> 87 + """Return the pushing event logs."""
> 88 + logs = []
> 89 + for e in events:
> 90 + logs.append('Pushing event %s to processor.' % e)
> 91 + return logs
>
> can be better read as:
>
> 86 + def _get_event_msgs(self, events):
> 87 + """Return the pushing event logs."""
> 88 + msg = 'Pushing event %s to processor.'
> 89 + return [msg % e for e in events]
>
> Anyways, if we use the former to create generators instead of lists in memory,
> we can improve all the helpers having only one like this:
>
> def _assert_logs(self, events):
> logs = []
>
> msg = 'Pushing event %r to processor.'
> logs.extend(msg % e for e in events)
>
> msg = 'Got from ReadDirectoryChangesW %r.'
> logs.extend(msg % [(WINDOWS_ACTIONS_NAMES[a], p) for a, p in actions]
> for actions in self.raw_events)
>
> msg = 'Is path %r a dir? %r'
> logs.extend(is_dir % data for data in self.paths_checked)
>
> self.assertTrue(self.memento.check_debug(*logs))
>
> * Also, always user %r to log variables, not %s, since the latter can fail for
> non-ascii strings.
>
> * For every addHanlder, you need to queue a removeHandler cleanup function.
>
> * The rest looks good!

All comments make sense and therefore I have removed all the helper functions, the only issue I found with the comments it he one regarding 'self.assertTrue(self.memento.check_debug(*logs))'. From a conversation in #chicharra with facu:

facundobatista> mandel, pong
 <mandel> facundobatista, can you give me a hand with the memento handler?
 <mandel> facundobatista, there is something I quite not get
 <facundobatista> mandel, yes
 <mandel> facundobatista, what does check_debug do when I pass a collection of expected logs messages?
 <facundobatista> mandel, it checks that *all* those messages are included in a single log line with debug level
 <mandel> facundobatista, ok, is there a method to check that all those logs messages have been added, but not in a single line?
 <mandel> or should I loop though them and do a check_dbug per message?
 <facundobatista> mandel, there's no method... the idea behind those multiple messages, is that you care to "some stuff being logged" (like the word "Starting", and the share id, etc), but you don't care the exact log wording
 <facundobatista> mandel, give me one example of what you're wanting
 <mandel> facundobatista, let me past the example
 <mandel> facundobatista, here: http://paste.ubuntu.com/707372/
 <mandel> facundobatista, I want to ensure that what I passed is indeed present in the handler
 <mandel> atm it does not work precisely for what you explained
 <mandel> I have no issue in using a loop, I was curious
 <facundobatista> mandel, we normally don't have a test case to check a zillion logs
 <facundobatista> mandel, we exercise that on certain situations, something is logged, always one line (or two)
 <facundobatista> mandel, so, for example, you should add a check for "Got from ReadDirectoryChangesW" in the test case that exercises the code that should be logging that

Since we really want to check that the events are logged correctly (we do not want to get the wrong names etc..) I have left the loop as it was.