Code review comment for lp:~mbp/bzr/externalbase

This:

195 - self.check_output(revs[1], 'cat-revision a@r-0-1')
196 - self.check_output(revs[2], 'cat-revision a@r-0-2')
197 - self.check_output(revs[3], 'cat-revision a@r-0-3')
198 -
199 - self.check_output(revs[1], 'cat-revision -r 1')
200 - self.check_output(revs[2], 'cat-revision -r 2')
201 - self.check_output(revs[3], 'cat-revision -r 3')
202 -
203 - self.check_output(revs[1], 'cat-revision -r revid:a@r-0-1')
204 - self.check_output(revs[2], 'cat-revision -r revid:a@r-0-2')
205 - self.check_output(revs[3], 'cat-revision -r revid:a@r-0-3')
206 + for i in [1, 2, 3]:
207 + self.assertEqual(revs[i],
208 + self.run_bzr('cat-revision -r revid:a@r-0-%d' % i)[0])
209 + self.assertEqual(revs[i],
210 + self.run_bzr('cat-revision a@r-0-%d' % i)[0])
211 + self.assertEqual(revs[i],
212 + self.run_bzr('cat-revision -r %d' % i)[0])

Results in harder to evaluate exceptions. I realise its very unlikely to fail but if it does you'll get something like
'serialised_revision' != 'other serialized revision'

and no evidence of *what* revision id we were actually asking for.

I'm going to land this as this code is pretty unlikely to change, but I think we should generally avoid such for loops vigorously: use parameterisation instead, or explicit lists as the test had before.