Code review comment for lp:~michael.nelson/launchpad/638090-base_version-property-for-differences

Hi Michael,

This branch looks good. I have included comments on the test changes that we already discussed on IRC.

-Edwin

>=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
>--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-09-22 11:03:20 +0000
>+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-10-06 14:44:02 +0000
>@@ -424,6 +422,38 @@
> self.assertContentEqual(
> ['source_pub', 'parent_source_pub'], cache)
>
>+ def test_base_version_none(self):
>+ # The attribute is set to None if there is no common base version.
>+ ds_diff = self.factory.makeDistroSeriesDifference()
>+
>+ self.assertIs(None, ds_diff.base_version)
>+
>+ def test_base_version_common(self):
>+ # The common base version is set when the difference is created.
>+ # Publish v1.0 of foo in both series.
>+ derived_series = self.factory.makeDistroSeries(
>+ parent_series=self.factory.makeDistroSeries())
>+ source_package_name = self.factory.getOrMakeSourcePackageName('foo')

The test should add multiple common versions to verify that it chooses
the right one (the latest one).

>+ self.factory.makeSourcePackagePublishingHistory(
>+ distroseries=derived_series,
>+ version='1.0',
>+ sourcepackagename=source_package_name,
>+ status = PackagePublishingStatus.PUBLISHED)

Remove spaces around "=".

>+ self.factory.makeSourcePackagePublishingHistory(
>+ distroseries=derived_series.parent_series,
>+ version='1.0',
>+ sourcepackagename=source_package_name,
>+ status = PackagePublishingStatus.PUBLISHED)

Same here.

>+ ds_diff = self.factory.makeDistroSeriesDifference(
>+ derived_series=derived_series, source_package_name_str='foo',
>+ versions={
>+ 'derived': '1.2',
>+ 'parent': '1.3',
>+ })
>+
>+ self.assertEqual('1.0', ds_diff.base_version)
>+
>
> class DistroSeriesDifferenceSourceTestCase(TestCaseWithFactory):