Code review comment for lp:~rvb/launchpad/dds-fix-localpackagediffs-745776

Lovely. A few minor comments.

[1]

+ @property
+ def parent_source_package_url(self):
+ return self._package_url(parent=True)
+
+ @property
+ def source_package_url(self):
+ return self._package_url()
+
+ def _package_url(self, parent=False):
+ if parent:
+ distro_series = self.context.derived_series.parent_series
+ version = self.context.parent_source_version
+ else:
+ distro_series = self.context.derived_series
+ version = self.context.source_version

You could get one less level of indirection if you pass distro_series
and version into _package_url():

    @property
    def parent_source_package_url(self):
        return self._package_url(
            self.context.derived_series.parent_series,
            self.context.parent_source_version)

    @property
    def source_package_url(self):
        return self._package_url(
            self.context.derived_series,
            self.context.source_version)

    def _package_url(self, distro_series, version):
        ...

But this is entirely up to you; it's fine as it is.

[2]

+ try:
+ url = canonical_url(
+ DistroSeriesSourcePackageRelease(
+ distro_series, pubs[0].sourcepackagerelease))
+ return url
+ except IndexError:
+ return ''

I assume you're attempting to catch any potential IndexErrors raised
by pubs[0], but it will also catch IndexError in any of the called
code.

Now pubs will be an SQLObjectResultSet, so you could do the following:

        from storm.zope.interfaces import IResultSet

        pub = IResultSet(pubs).first()
        if pub is None:
            return ''
        else:
            return canonical_url(
                DistroSeriesSourcePackageRelease(
                    distro_series, pub.sourcepackagerelease))

Out of interest, why do you only consider the first publication here?

[3]

+ row = diff_table.tbody.findAll('tr')[0]

This could simplified to:

        row = diff_table.tbody.tr

[4]

+ self.assertTrue(link.endswith(new_version))
...
+ self.assertTrue(
+ links[0].get('href').endswith(difference.source_version))

In the event of a failure, you could make the report more informative
by using the EndsWith matcher:

    from testtools.matchers import EndsWith
    self.assertTrue(link, EndsWith(new_version))