Code review comment for lp://staging/~gary/launchpad/bug875697

Revision history for this message
Ian Booth (wallyworld) wrote :

Hi Gary

This looks like a nice change. I'll take it as a given that when the code calls page = client.load_page(), the page.content can be parsed onto a dict containing keys testCase, testName, type. If any one of those keys are missing, then:

76 + try:
77 + msg += (' The last test that ran to '
78 + 'completion before timing out was '
79 + '%(testCase)s:%(testName)s. The test %(type)sed.'
80 + % self._last_test_info)
81 + except KeyError:
82 + pass

will not append anything to msg. Do we need to allow for that and still append a useful message fragment? Or is the test report dict always guaranteed to have all the required keys?

Should there be any tests for this stuff? I would like to see some but I'll leave it to you.

review: Approve (code)

« Back to merge proposal