Merge lp:~stevenk/launchpad/destroy-pppd into lp:launchpad

[Steve Kowalik]

Destroy the process-pending-packagediffs cronjob, its tests, the model code it used, and the horses they rode in on.

Destroy the process-pending-packagediffs cronjob and friends, now it is no longer needed due to the switch to the job system.

[William Grant (wgrant)]

review: Approve (code)

=== removed file 'cronscripts/process-pending-packagediffs.py'
--- cronscripts/process-pending-packagediffs.py	2013-01-07 02:40:55 +0000
+++ cronscripts/process-pending-packagediffs.py	1970-01-01 00:00:00 +0000
@@ -1,35 +0,0 @@
-#!/usr/bin/python -S
-#
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Process pending PackageDiffs.
-
-Process a optionally limited set of pending PackageDiffs.
-
-By default it process up to 50 diffs each run, which is enough to catch
-up on 1 hour of uploads (on average).
-
-However users might benefit for more frequently runs since the diff ETA
-relative to the upload will be shorter.
-
-The cycle time needs to be balanced with the run time to produce the shortest
-diff ETA and to not overlap much, for instance, if it has to diff monster
-sources like openoffice or firefox.
-
-Experiments with the cycle time will be safe enough and won't sink the host
-performance, since the lock file is exclusive.
-"""
-
-__metaclass__ = type
-
-import _pythonpath
-
-from lp.services.config import config
-from lp.soyuz.scripts.packagediff import ProcessPendingPackageDiffs
-
-
-if __name__ == '__main__':
-    script = ProcessPendingPackageDiffs(
-        'process-pending-packagediffs', dbuser=config.uploader.dbuser)
-    script.lock_and_run()

=== modified file 'lib/lp/soyuz/doc/package-diff.txt'
--- lib/lp/soyuz/doc/package-diff.txt	2013-01-08 07:54:50 +0000
+++ lib/lp/soyuz/doc/package-diff.txt	2013-07-31 00:41:29 +0000
@@ -117,8 +117,7 @@
 Before starting let's enable the universe component and add the i386
 chroot in hoary in order to be able to accept the NEW packages.
 
-    >>> from lp.soyuz.model.component import (
-    ...     ComponentSelection)
+    >>> from lp.soyuz.model.component import ComponentSelection
     >>> from lp.services.librarian.model import LibraryFileAlias
     >>> from lp.soyuz.interfaces.component import IComponentSet
 
@@ -154,8 +153,7 @@
 
     >>> packager.buildUpstream()
     >>> packager.buildSource(signed=False)
-    >>> biscuit_one_pub = packager.uploadSourceVersion(
-    ...     '1.0-1', policy='sync')
+    >>> biscuit_one_pub = packager.uploadSourceVersion('1.0-1', policy='sync')
 
     >>> len(biscuit_one_pub.sourcepackagerelease.package_diffs)
     0
@@ -436,29 +434,6 @@
     biscuit diff from 1.0-1 to 1.0-8               True   -6
     pmount diff from 0.1-1 to 0.1-2                False  -7
 
-Or only the PackageDiffs not yet fullfilled.
-
-    >>> print_diffs(packagediff_set.getPendingDiffs())
-    pmount diff from 0.1-1 to 0.1-2                False   0
-    biscuit diff from 1.0-8 to 1.0-9               False   2
-    biscuit diff from 1.0-8 to 1.0-12              False   3
-    biscuit diff from 1.0-9 to 1.0-10              False   4
-    biscuit diff from 1.0-8 to 1.0-11              False   5
-    biscuit diff from 1.0-8 (in Ubuntu) to 1.0-2   False   6
-    biscuit diff from 1.0-2 to 1.0-3               False   7
-
-Note that the iteration over all PackageDiffs is sorted by descending
-database ID, i.e. newest first, and getPendingDiffs() results are
-ordered by ascending database IDs, oldest first.
-
-getPendingDiffs() results can optionally be limited.
-
-    >>> packagediff_set.getPendingDiffs().count()
-    7
-
-    >>> packagediff_set.getPendingDiffs(limit=2).count()
-    2
-
 All package diffs targeting a set of source package releases can also
 be requested.  The results are ordered by the source package release
 ID:

=== modified file 'lib/lp/soyuz/interfaces/packagediff.py'
--- lib/lp/soyuz/interfaces/packagediff.py	2013-01-07 02:40:55 +0000
+++ lib/lp/soyuz/interfaces/packagediff.py	2013-07-31 00:41:29 +0000
@@ -92,14 +92,6 @@
     def get(diff_id):
         """Retrieve a `PackageDiff` for the given id."""
 
-    def getPendingDiffs(limit=None):
-        """Return all pending `PackageDiff` records.
-
-        :param limit: optional results limitation.
-
-        :return a `SelectResult` ordered by id respecting the given limit.
-        """
-
     def getDiffsToReleases(sprs, preload_for_display=False):
         """Return all diffs that targetting a set of source package releases.
 

=== modified file 'lib/lp/soyuz/model/packagediff.py'
--- lib/lp/soyuz/model/packagediff.py	2013-07-25 12:39:54 +0000
+++ lib/lp/soyuz/model/packagediff.py	2013-07-31 00:41:29 +0000
@@ -264,12 +264,6 @@
         """See `IPackageDiffSet`."""
         return PackageDiff.get(diff_id)
 
-    def getPendingDiffs(self, limit=None):
-        return IStore(PackageDiff).find(
-            PackageDiff,
-            PackageDiff.status == PackageDiffStatus.PENDING).order_by(
-                PackageDiff.id).config(limit=limit)
-
     def getDiffsToReleases(self, sprs, preload_for_display=False):
         """See `IPackageDiffSet`."""
         from lp.registry.model.distribution import Distribution

=== removed file 'lib/lp/soyuz/scripts/packagediff.py'
--- lib/lp/soyuz/scripts/packagediff.py	2010-08-20 20:31:18 +0000
+++ lib/lp/soyuz/scripts/packagediff.py	1970-01-01 00:00:00 +0000
@@ -1,62 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""PackageDiff cronscript class."""
-
-__metaclass__ = type
-
-__all__ = [
-    'ProcessPendingPackageDiffs',
-    ]
-
-from zope.component import getUtility
-
-from lp.services.scripts.base import (
-    LaunchpadCronScript,
-    LaunchpadScriptFailure,
-    )
-from lp.soyuz.interfaces.packagediff import IPackageDiffSet
-
-
-class ProcessPendingPackageDiffs(LaunchpadCronScript):
-
-    def add_my_options(self):
-        # 50 diffs seems to be more them enough to process all uploaded
-        # source packages for 1 hour (average upload rate) for ubuntu
-        # primary archive, security and PPAs in general.
-        self.parser.add_option(
-            "-l", "--limit", type="int", default=50,
-            help="Maximum number of requests to be processed in this run.")
-
-        self.parser.add_option(
-            "-n", "--dry-run",
-            dest="dryrun", action="store_true", default=False,
-            help="Whether or not to commit the transaction.")
-
-    def main(self):
-        """Process pending `PackageDiff` records.
-
-        Collect up to the maximum number of pending `PackageDiff` records
-        available and process them.
-
-        Processed diffs results are commited individually.
-        """
-        if self.args:
-            raise LaunchpadScriptFailure("Unhandled arguments %r" % self.args)
-
-        packagediff_set = getUtility(IPackageDiffSet)
-
-        pending_diffs = packagediff_set.getPendingDiffs(
-            limit=self.options.limit)
-        self.logger.debug(
-            'Considering %s diff requests' % pending_diffs.count())
-
-        # Iterate over all pending packagediffs.
-        for packagediff in pending_diffs:
-            self.logger.debug(
-                'Performing package diff for %s from %s' % (
-                    packagediff.from_source.name, packagediff.title))
-            packagediff.performDiff()
-            if not self.options.dryrun:
-                self.logger.debug('Commiting the transaction.')
-                self.txn.commit()

=== removed file 'lib/lp/soyuz/scripts/tests/test_processpendingpackagediffs.py'
--- lib/lp/soyuz/scripts/tests/test_processpendingpackagediffs.py	2012-01-01 02:58:52 +0000
+++ lib/lp/soyuz/scripts/tests/test_processpendingpackagediffs.py	1970-01-01 00:00:00 +0000
@@ -1,106 +0,0 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-__metaclass__ = type
-
-import os
-import subprocess
-import sys
-
-from lp.services.config import config
-from lp.services.log.logger import BufferLogger
-from lp.soyuz.scripts.packagediff import ProcessPendingPackageDiffs
-from lp.soyuz.tests.soyuz import TestPackageDiffsBase
-from lp.testing.layers import LaunchpadZopelessLayer
-
-
-class TestProcessPendingPackageDiffsScript(TestPackageDiffsBase):
-    """Test the process-pending-packagediffs.py script."""
-    layer = LaunchpadZopelessLayer
-    dbuser = config.uploader.dbuser
-
-    def runProcessPendingPackageDiffs(self, extra_args=None):
-        """Run process-pending-packagediffs.py.
-
-        Returns a tuple of the process's return code, stdout output and
-        stderr output."""
-        if extra_args is None:
-            extra_args = []
-        script = os.path.join(
-            config.root, "cronscripts", "process-pending-packagediffs.py")
-        args = [sys.executable, script]
-        args.extend(extra_args)
-        process = subprocess.Popen(
-            args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        stdout, stderr = process.communicate()
-        return (process.returncode, stdout, stderr)
-
-    def testSimpleScriptRun(self):
-        """Try a simple processing-pending-packagediffs.py run."""
-        returncode, out, err = self.runProcessPendingPackageDiffs()
-        if returncode != 0:
-            print "\nStdout:\n%s\nStderr\n%s\n" % (out, err)
-        self.assertEqual(0, returncode)
-
-        self.layer.txn.abort()
-
-        # The pending PackageDiff request was processed.
-        self.assertEqual(self.getPendingDiffs().count(), 0)
-
-    def getDiffProcessor(self, limit=None):
-        """Return a `ProcessPendingPackageDiffs` instance.
-
-        :param limit: if passed, it will be used as the 'limit' script
-           argument.
-
-        :return the initialized script object using `BufferLogger` and
-           the given parameters.
-        """
-        test_args = []
-        if limit is not None:
-            test_args.append('-l %s' % limit)
-
-        diff_processor = ProcessPendingPackageDiffs(
-            name='process-pending-packagediffs', test_args=test_args)
-        diff_processor.logger = BufferLogger()
-        diff_processor.txn = self.layer.txn
-        return diff_processor
-
-    def testSimpleRun(self):
-        """Simple run of the script class.
-
-        The only diff available is processed after its run.
-        """
-        # Setup a DiffProcessor.
-        diff_processor = self.getDiffProcessor()
-        diff_processor.main()
-
-        # The pending PackageDiff request was processed.
-        # See doc/package-diff.txt for more information.
-        pending_diffs = self.getPendingDiffs()
-        self.assertEqual(pending_diffs.count(), 0)
-
-    def testLimitedRun(self):
-        """Run the script with a limited scope.
-
-        Check if a limited run of the script only processes up to 'limit'
-        pending diff records and exits.
-        """
-        # Setup a DiffProcessor limited to one request per run.
-        diff_processor = self.getDiffProcessor(limit=1)
-
-        # Upload a new source version, so we have two pending PackageDiff
-        # records to process.
-        self.packager.buildVersion('1.0-3', changelog_text="biscuits")
-        self.packager.buildSource(include_orig=False)
-        self.packager.uploadSourceVersion('1.0-3', suite="warty-updates")
-        self.assertEqual(self.getPendingDiffs().count(), 2)
-
-        # The first processor run will process only one PackageDiff,
-        # the other will remain.
-        diff_processor.main()
-        self.assertEqual(self.getPendingDiffs().count(), 1)
-
-        # The next run process the remaining one.
-        diff_processor.main()
-        self.assertEqual(self.getPendingDiffs().count(), 0)

=== modified file 'lib/lp/soyuz/stories/soyuz/xx-package-diff.txt'
--- lib/lp/soyuz/stories/soyuz/xx-package-diff.txt	2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/stories/soyuz/xx-package-diff.txt	2013-07-31 00:41:29 +0000
@@ -73,24 +73,13 @@
     >>> diff_one = biscuit_one_pub.sourcepackagerelease.requestDiffTo(
     ...     requester=name16,
     ...     to_sourcepackagerelease=biscuit_two_pub.sourcepackagerelease)
-
     >>> diff_two = biscuit_two_pub.sourcepackagerelease.requestDiffTo(
     ...     requester=name16,
     ...     to_sourcepackagerelease=biscuit_three_pub.sourcepackagerelease)
-
     >>> diff_three = biscuit_three_pub.sourcepackagerelease.requestDiffTo(
     ...     requester=name16,
     ...     to_sourcepackagerelease=biscuit_four_pub.sourcepackagerelease)
 
-    >>> from lp.soyuz.interfaces.packagediff import IPackageDiffSet
-    >>> diff_set = getUtility(IPackageDiffSet)
-    >>> diffs = list(diff_set.getPendingDiffs())
-    >>> for diff in diffs:
-    ...     print diff.title
-    diff from 1.0-1 to 1.0-2
-    diff from 1.0-2 to 1.0-3
-    diff from 1.0-3 (in Ubuntu) to 1.0-4
-
 Perform some diffs in advance, the first diff in ubuntu and the diff
 in PPA will be performed, the second diff in ubuntu will be performed
 later.
@@ -166,9 +155,7 @@
 the missing link is rendered.
 
     >>> login('[e-mail]')
-    >>> diff_set = getUtility(IPackageDiffSet)
-    >>> [diff] = diff_set.getPendingDiffs()
-    >>> perform_fake_diff(diff, 'biscuit_1.0-2_1.0-3.diff.gz')
+    >>> perform_fake_diff(diff_two, 'biscuit_1.0-2_1.0-3.diff.gz')
     >>> transaction.commit()
     >>> logout()
 

=== modified file 'lib/lp/soyuz/tests/soyuz.py'
--- lib/lp/soyuz/tests/soyuz.py	2012-07-12 23:47:04 +0000
+++ lib/lp/soyuz/tests/soyuz.py	2013-07-31 00:41:29 +0000
@@ -7,36 +7,21 @@
 
 __all__ = [
     'SoyuzTestHelper',
-    'TestPackageDiffsBase',
     ]
 
-import unittest
-
 from zope.component import getUtility
 
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.services.config import config
-from lp.services.librarian.model import LibraryFileAlias
 from lp.soyuz.enums import PackagePublishingStatus
-from lp.soyuz.interfaces.packagediff import IPackageDiffSet
 from lp.soyuz.model.publishing import (
     BinaryPackagePublishingHistory,
     SourcePackagePublishingHistory,
     )
-from lp.soyuz.tests.fakepackager import FakePackager
-from lp.testing.dbuser import dbuser
-from lp.testing.gpgkeys import import_public_test_keys
-from lp.testing.layers import LaunchpadZopelessLayer
 from lp.testing.sampledata import (
     BUILDD_ADMIN_USERNAME,
-    CHROOT_LIBRARYFILEALIAS,
-    I386_ARCHITECTURE_NAME,
-    LAUNCHPAD_DBUSER_NAME,
     UBUNTU_DISTRIBUTION_NAME,
-    WARTY_DISTROSERIES_NAME,
-    WARTY_UPDATES_SUITE_NAME,
     )
 
 
@@ -137,69 +122,3 @@
         Return True if the lists matches, otherwise False.
         """
         return [p.id for p in expected] == [r.id for r in given]
-
-
-class TestPackageDiffsBase(unittest.TestCase):
-    """Base class facilitating tests related to package diffs."""
-    layer = LaunchpadZopelessLayer
-    dbuser = config.uploader.dbuser
-
-    def setUp(self):
-        """Setup proper DB connection and contents for tests
-
-        Connect to the DB as the 'uploader' user (same user used in the
-        script), upload the test packages (see `uploadTestPackages`) and
-        commit the transaction.
-
-        Store the `FakePackager` object used in the test uploads as `packager`
-        so the tests can reuse it if necessary.
-        """
-        super(TestPackageDiffsBase, self).setUp()
-        with dbuser(LAUNCHPAD_DBUSER_NAME):
-            fake_chroot = LibraryFileAlias.get(CHROOT_LIBRARYFILEALIAS)
-            ubuntu = getUtility(IDistributionSet).getByName(
-                UBUNTU_DISTRIBUTION_NAME)
-            warty = ubuntu.getSeries(WARTY_DISTROSERIES_NAME)
-            warty[I386_ARCHITECTURE_NAME].addOrUpdateChroot(fake_chroot)
-
-        self.packager = self.uploadTestPackages()
-        self.layer.txn.commit()
-
-    def uploadTestPackages(self):
-        """Upload packages for testing `PackageDiff` generation script.
-
-        Upload zeca_1.0-1 and zeca_1.0-2 sources, so a `PackageDiff` between
-        them is created.
-
-        Assert there is not pending `PackageDiff` in the DB before uploading
-        the package and also assert that there is one after the uploads.
-
-        :return: the FakePackager object used to generate and upload the test,
-            packages, so the tests can upload subsequent version if necessary.
-        """
-        # No pending PackageDiff available in sampledata.
-        self.assertEqual(self.getPendingDiffs().count(), 0)
-
-        import_public_test_keys()
-        # Use FakePackager to upload a base package to ubuntu.
-        packager = FakePackager(
-            'zeca', '1.0', '[e-mail]')
-        packager.buildUpstream()
-        packager.buildSource()
-        packager.uploadSourceVersion('1.0-1', suite=WARTY_UPDATES_SUITE_NAME)
-
-        # Upload a new version of the source, so a PackageDiff can
-        # be created.
-        packager.buildVersion('1.0-2', changelog_text="cookies")
-        packager.buildSource(include_orig=False)
-        packager.uploadSourceVersion('1.0-2', suite=WARTY_UPDATES_SUITE_NAME)
-
-        # Check if there is exactly one pending PackageDiff record and
-        # It's the one we have just created.
-        self.assertEqual(self.getPendingDiffs().count(), 1)
-
-        return packager
-
-    def getPendingDiffs(self):
-        """Pending `PackageDiff` available."""
-        return getUtility(IPackageDiffSet).getPendingDiffs()

=== modified file 'lib/lp/soyuz/tests/test_packagediff.py'
--- lib/lp/soyuz/tests/test_packagediff.py	2013-07-23 17:57:52 +0000
+++ lib/lp/soyuz/tests/test_packagediff.py	2013-07-31 00:41:29 +0000
@@ -6,7 +6,9 @@
 __metaclass__ = type
 
 from datetime import datetime
+import os.path
 
+import transaction
 from zope.security.proxy import removeSecurityProxy
 
 from lp.services.config import config
@@ -16,13 +18,44 @@
 from lp.services.job.model.job import Job
 from lp.soyuz.enums import PackageDiffStatus
 from lp.soyuz.model.archive import Archive
-from lp.soyuz.tests.soyuz import TestPackageDiffsBase
 from lp.testing import TestCaseWithFactory
 from lp.testing.dbuser import dbuser
 from lp.testing.layers import LaunchpadZopelessLayer
 
 
-class TestPackageDiffs(TestPackageDiffsBase, TestCaseWithFactory):
+def create_proper_job(factory):
+    archive = factory.makeArchive()
+    foo_dash1 = factory.makeSourcePackageRelease(archive=archive)
+    foo_dash15 = factory.makeSourcePackageRelease(archive=archive)
+    suite_dir = 'lib/lp/archiveuploader/tests/data/suite'
+    files = {
+        '%s/foo_1.0-1/foo_1.0-1.diff.gz' % suite_dir: None,
+        '%s/foo_1.0-1/foo_1.0-1.dsc' % suite_dir: None,
+        '%s/foo_1.0-1/foo_1.0.orig.tar.gz' % suite_dir: None,
+        '%s/foo_1.0-1.5/foo_1.0-1.5.diff.gz' % suite_dir: None,
+        '%s/foo_1.0-1.5/foo_1.0-1.5.dsc' % suite_dir: None}
+    for name in files:
+        filename = os.path.split(name)[-1]
+        with open(name, 'r') as content:
+            files[name] = factory.makeLibraryFileAlias(
+                filename=filename, content=content.read())
+    transaction.commit()
+    dash1_files = (
+        '%s/foo_1.0-1/foo_1.0-1.diff.gz' % suite_dir,
+        '%s/foo_1.0-1/foo_1.0-1.dsc' % suite_dir,
+        '%s/foo_1.0-1/foo_1.0.orig.tar.gz' % suite_dir)
+    dash15_files = (
+        '%s/foo_1.0-1/foo_1.0.orig.tar.gz' % suite_dir,
+        '%s/foo_1.0-1.5/foo_1.0-1.5.diff.gz' % suite_dir,
+        '%s/foo_1.0-1.5/foo_1.0-1.5.dsc' % suite_dir)
+    for name in dash1_files:
+        foo_dash1.addFile(files[name])
+    for name in dash15_files:
+        foo_dash15.addFile(files[name])
+    return foo_dash1.requestDiffTo(factory.makePerson(), foo_dash15)
+
+
+class TestPackageDiffs(TestCaseWithFactory):
     """Test package diffs."""
     layer = LaunchpadZopelessLayer
     dbuser = config.uploader.dbuser
@@ -30,7 +63,7 @@
     def test_packagediff_working(self):
         # Test the case where none of the files required for the diff are
         # expired in the librarian and where everything works as expected.
-        [diff] = self.getPendingDiffs()
+        diff = create_proper_job(self.factory)
         self.assertEqual(0, removeSecurityProxy(diff)._countDeletedLFAs())
         diff.performDiff()
         self.assertEqual(PackageDiffStatus.COMPLETED, diff.status)
@@ -39,11 +72,7 @@
         """Expire the files associated with the given source package in the
         librarian."""
         assert expire or delete
-        store = IStore(Archive)
-        query = """
-            UPDATE LibraryFileAlias lfa
-            SET
-            """
+        query = "UPDATE LibraryFileAlias lfa SET "
         if expire:
             query += "expires = %s" % sqlvalues(datetime.utcnow())
         if expire and delete:
@@ -59,45 +88,36 @@
                 AND sprf.libraryfile = lfa.id
             """ % sqlvalues(source.id)
         with dbuser('launchpad'):
-            store.execute(query)
+            IStore(Archive).execute(query)
 
     def test_packagediff_with_expired_and_deleted_lfas(self):
         # Test the case where files required for the diff are expired *and*
         # deleted in the librarian causing a package diff failure.
-        [diff] = self.getPendingDiffs()
-        # Expire and delete the files associated with the 'from_source'
-        # package.
+        diff = create_proper_job(self.factory)
         self.expireLFAsForSource(diff.from_source)
-        # The helper method now finds 3 expired files.
-        self.assertEqual(3, removeSecurityProxy(diff)._countDeletedLFAs())
+        self.assertEqual(4, removeSecurityProxy(diff)._countDeletedLFAs())
         diff.performDiff()
-        # The diff fails due to the presence of expired files.
         self.assertEqual(PackageDiffStatus.FAILED, diff.status)
 
     def test_packagediff_with_expired_but_not_deleted_lfas(self):
         # Test the case where files required for the diff are expired but
         # not deleted in the librarian still allowing the package diff to be
         # performed.
-        [diff] = self.getPendingDiffs()
+        diff = create_proper_job(self.factory)
         # Expire but don't delete the files associated with the 'from_source'
         # package.
         self.expireLFAsForSource(diff.from_source, expire=True, delete=False)
-        # The helper method now finds no expired files.
         self.assertEqual(0, removeSecurityProxy(diff)._countDeletedLFAs())
         diff.performDiff()
-        # The diff succeeds as expected.
         self.assertEqual(PackageDiffStatus.COMPLETED, diff.status)
 
     def test_packagediff_with_deleted_but_not_expired_lfas(self):
         # Test the case where files required for the diff have been
         # deleted explicitly, not through expiry.
-        [diff] = self.getPendingDiffs()
-        # Delete the files associated with the 'from_source' package.
+        diff = create_proper_job(self.factory)
         self.expireLFAsForSource(diff.from_source, expire=False, delete=True)
-        # The helper method now finds 3 expired files.
-        self.assertEqual(3, removeSecurityProxy(diff)._countDeletedLFAs())
+        self.assertEqual(4, removeSecurityProxy(diff)._countDeletedLFAs())
         diff.performDiff()
-        # The diff fails due to the presence of expired files.
         self.assertEqual(PackageDiffStatus.FAILED, diff.status)
 
     def test_packagediff_private_with_copied_spr(self):
@@ -128,7 +148,11 @@
         self.assertFalse(diff.private)
 
     def test_job_created(self):
-        # The setup code already creates a packagediff.
+        # Requesting a package diff creates a PackageDiffJob.
+        ppa = self.factory.makeArchive()
+        from_spr = self.factory.makeSourcePackageRelease(archive=ppa)
+        to_spr = self.factory.makeSourcePackageRelease(archive=ppa)
+        from_spr.requestDiffTo(ppa.owner, to_spr)
         [job] = IStore(Job).find(
             Job, Job.base_job_type == JobType.GENERATE_PACKAGE_DIFF)
         self.assertIsNot(None, job)

=== modified file 'lib/lp/soyuz/tests/test_packagediffjob.py'
--- lib/lp/soyuz/tests/test_packagediffjob.py	2013-07-23 17:57:52 +0000
+++ lib/lp/soyuz/tests/test_packagediffjob.py	2013-07-31 00:41:29 +0000
@@ -3,8 +3,6 @@
 
 __metaclass__ = type
 
-import os.path
-
 from testtools.content import text_content
 import transaction
 from zope.component import getUtility
@@ -19,6 +17,7 @@
     IPackageDiffJobSource,
     )
 from lp.soyuz.model.packagediffjob import PackageDiffJob
+from lp.soyuz.tests.test_packagediff import create_proper_job
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
 from lp.testing import (
@@ -34,38 +33,6 @@
     )
 
 
-def create_proper_job(factory):
-    archive = factory.makeArchive()
-    foo_dash1 = factory.makeSourcePackageRelease(archive=archive)
-    foo_dash15 = factory.makeSourcePackageRelease(archive=archive)
-    suite_dir = 'lib/lp/archiveuploader/tests/data/suite'
-    files = {
-        '%s/foo_1.0-1/foo_1.0-1.diff.gz' % suite_dir: None,
-        '%s/foo_1.0-1/foo_1.0-1.dsc' % suite_dir: None,
-        '%s/foo_1.0-1/foo_1.0.orig.tar.gz' % suite_dir: None,
-        '%s/foo_1.0-1.5/foo_1.0-1.5.diff.gz' % suite_dir: None,
-        '%s/foo_1.0-1.5/foo_1.0-1.5.dsc' % suite_dir: None}
-    for name in files:
-        filename = os.path.split(name)[-1]
-        with open(name, 'r') as content:
-            files[name] = factory.makeLibraryFileAlias(
-                filename=filename, content=content.read())
-    transaction.commit()
-    dash1_files = (
-        '%s/foo_1.0-1/foo_1.0-1.diff.gz' % suite_dir,
-        '%s/foo_1.0-1/foo_1.0-1.dsc' % suite_dir,
-        '%s/foo_1.0-1/foo_1.0.orig.tar.gz' % suite_dir)
-    dash15_files = (
-        '%s/foo_1.0-1/foo_1.0.orig.tar.gz' % suite_dir,
-        '%s/foo_1.0-1.5/foo_1.0-1.5.diff.gz' % suite_dir,
-        '%s/foo_1.0-1.5/foo_1.0-1.5.dsc' % suite_dir)
-    for name in dash1_files:
-        foo_dash1.addFile(files[name])
-    for name in dash15_files:
-        foo_dash15.addFile(files[name])
-    return foo_dash1.requestDiffTo(factory.makePerson(), foo_dash15)
-
-
 class TestPackageDiffJob(TestCaseWithFactory):
 
     layer = LaunchpadZopelessLayer