Merge lp:~jml/pkgme-service/actually-run-pkgme into lp:pkgme-service

[Jonathan Lange]

At long last, actually running pkgme-binary from pkgme-service.

I'm a little too zonked to provide much more details right now, but happy to answer questions.

[James Westby (james-w)]

Hi,

This broadly looks good, and is certainly an improvement.

As it stands this test will only work when run from source:

437 + import acceptance

as pkgme-binary doesn't include this when installed. We could move
it in to the devportalbinary namespace?

Thanks,

James

review: Approve

[Jonathan Lange (jml)]

Merged. Filed bug 893675, bug 893674 and bug 893563.

=== modified file 'fabtasks/django.py'
--- fabtasks/django.py	2011-08-30 17:41:11 +0000
+++ fabtasks/django.py	2011-11-22 14:09:24 +0000
@@ -1,6 +1,3 @@
-import os
-import os.path
-
 from fabric.api import local
 
 

=== modified file 'setup.py'
--- setup.py	2011-11-09 11:14:13 +0000
+++ setup.py	2011-11-22 14:09:24 +0000
@@ -1,22 +1,23 @@
 from setuptools import setup, find_packages
 
 setup(
-    name = "django-pkgme",
-    version = "0.1",
-    author = "Canonical Consumer Applications",
-    author_email = "[e-mail]",
-    license = "AGPL3",
-    install_requires = [
+    name="django-pkgme",
+    version="0.1",
+    author="Canonical Consumer Applications",
+    author_email="[e-mail]",
+    license="AGPL3",
+    install_requires=[
         'django==1.3',
         'django-piston',
         # XXX: Until https://github.com/ask/django-celery/pull/88 is fixed, we
         # have to use the older release. Review this when django-celery
         # releases the version after 2.4.1. -- jml
         'django-celery==2.4.0',
+        'fixtures',
         'south==0.7.3',
         'oauth',
-    ],
-    zip_safe = False,
-    packages = find_packages('src'),
-    package_dir = {'': 'src'},
+        ],
+    zip_safe=False,
+    packages=find_packages('src'),
+    package_dir={'': 'src'},
 )

=== modified file 'src/djpkgme/models.py'
--- src/djpkgme/models.py	2011-09-01 21:46:47 +0000
+++ src/djpkgme/models.py	2011-11-22 14:09:24 +0000
@@ -2,6 +2,11 @@
 from django.db import models
 from datetime import datetime
 
+from devportalbinary.binary import (
+    CATEGORIES,
+    PACKAGE_NAME,
+    TAGLINE,
+    )
 
 class AppMetadata(models.Model):
     # ID of the app, according to myapps
@@ -16,8 +21,12 @@
     categories = models.CharField(max_length=128, blank=True)
     department = models.CharField(max_length=64, blank=True)
     keywords = models.TextField(blank=True, default='')
+    # XXX: We *think* this is the URL to the tarball that contains the app
+    # that we are packaging.
     package_url = models.CharField(max_length=256)
     icon_url = models.CharField(max_length=256)
+    # XXX: This should not be required, or even sent. It's not part of the
+    # package.
     screenshot_url = models.CharField(max_length=256)
     price = models.DecimalField(max_digits=7, decimal_places=2, null=True)
     callback_url = models.CharField(max_length=256)
@@ -35,6 +44,18 @@
     # Location of the built package
     packaged_app_url = models.CharField(max_length=256)
 
+    def as_dict(self):
+        """Return this metadata as a dict.
+
+        The contents of that dict should be suitable for dumping as JSON into
+        devportal-metadata.json (see lp:pkgme-binary).
+        """
+        return {
+            CATEGORIES: self.categories,
+            PACKAGE_NAME: self.package_name,
+            TAGLINE: self.tagline,
+            }
+
     def update_guesses(self, guesses):
         """Updates the guess fields based on a result from pkgme."""
         other = {}

=== modified file 'src/djpkgme/tasks.py'
--- src/djpkgme/tasks.py	2011-09-01 21:46:47 +0000
+++ src/djpkgme/tasks.py	2011-11-22 14:09:24 +0000
@@ -1,4 +1,8 @@
 import json
+import os
+import shutil
+import tarfile
+
 from celery.registry import tasks
 from celery.task import Task
 from django.conf import settings
@@ -10,7 +14,13 @@
     OAuthSignatureMethod_PLAINTEXT,
     )
 
-import djpkgme.mock_pkgme as pkgme
+from devportalbinary.binary import METADATA_FILE
+from devportalbinary.database import download_file
+from fixtures import TempDir
+from pkgme import write_packaging
+from pkgme.debuild import build_source_package
+
+from djpkgme import mock_pkgme
 
 
 def oauth_sign_request(url):
@@ -34,7 +44,7 @@
 
 class GuessPackagingDetailsTask(Task):
     def run(self, metadata):
-        result = pkgme.guess(metadata)
+        result = mock_pkgme.guess(metadata)
         metadata.update_guesses(result)
         submit_pkgme_info(metadata)
         return result
@@ -42,8 +52,39 @@
 
 
 class BuildPackageTask(Task):
+
+    def build_package(self, metadata, overrides, output_tar_path):
+        """Build packaging for 'metadata' and store it all in 'output_tar_path'.
+        """
+        with TempDir() as temp_dir:
+            # Download, extract and remove the tarball.
+            output_dir = temp_dir.path
+            path = download_file(metadata.package_url, output_dir)
+            # XXX: 'package' is an arbitrary name. Maybe we should pick a
+            # better arbitrary name?
+            extraction_path = os.path.join(output_dir, 'package')
+            with tarfile.open(path) as t:
+                t.extractall(extraction_path)
+            os.unlink(path)
+            # Write the metadata out to a special file, where the binary
+            # backend will find it.
+            metadata_filename = os.path.join(extraction_path, METADATA_FILE)
+            with open(metadata_filename, 'w') as metadata_file:
+                json.dump(metadata.as_dict(), metadata_file)
+            # Run pkgme: generate the packaging and build a source package.
+            write_packaging(extraction_path, allowed_backend_names=['binary'])
+            build_source_package(extraction_path, False)
+            # Create a new tarball containing the source package and the
+            # metadata file.
+            os.rename(
+                metadata_filename, os.path.join(output_dir, METADATA_FILE))
+            shutil.rmtree(extraction_path)
+            with tarfile.open(output_tar_path, 'w') as t:
+                for name in os.listdir(output_dir):
+                    t.add(os.path.join(output_dir, name), name)
+
     def run(self, metadata, overrides):
-        result = pkgme.build_package(metadata, overrides)
+        result = self.build_package(metadata, overrides)
         metadata.packaged_app_url = result
         metadata.save()
         submit_pkgme_info(metadata)

=== modified file 'src/djpkgme/tests/factory.py'
--- src/djpkgme/tests/factory.py	2011-09-01 20:35:22 +0000
+++ src/djpkgme/tests/factory.py	2011-11-22 14:09:24 +0000
@@ -1,9 +1,24 @@
-from django.test import TestCase
 from itertools import count
-
+import os
+
+from django.test import TestCase as DjangoTestCase
+from fixtures import (
+    EnvironmentVariableFixture,
+    Fixture,
+    TempDir,
+    )
+from testtools import (
+    RunTest,
+    TestCase,
+    )
+
+from devportalbinary.database import PackageDatabase
 from djpkgme.models import AppMetadata
 
-__all__ = ['TestCaseWithFactory']
+__all__ = [
+    'EmptyBinaryPackageDatabase',
+    'TestCaseWithFactory',
+    ]
 
 
 class PkgmeObjectFactory(object):
@@ -20,7 +35,9 @@
         return prefix + str(self.get_unique_integer())
 
     def make_metadata(self, myapps_id=None, callback_url=None,
-        with_guesses=False):
+        with_guesses=False, package_name=None):
+        if package_name is None:
+            package_name = self.get_unique_string('package-name')
         if myapps_id is None:
             myapps_id = self.get_unique_integer()
         if callback_url is None:
@@ -39,7 +56,43 @@
         return AppMetadata.objects.create(myapps_id=myapps_id,
             executable=executable, distroarchseries=distroarchseries,
             dependencies=dependencies, other_guesses=other_guesses,
-            callback_url=callback_url)
+            callback_url=callback_url, package_name=package_name)
+
+
+class DjangoRunner(RunTest):
+    """Use with run_tests_with to run Django tests.
+
+    Django's test case has a couple of hooks that it runs before and after
+    tests to isolate transactions and do other fun things
+    <https://docs.djangoproject.com/en/dev/topics/testing/#django.test.TestCase>.
+
+    Use this runner to get that same effect with testtools.
+    """
+
+    class MyDjangoTestCase(DjangoTestCase):
+        def test_foo(self):
+            pass
+
+    def __init__(self, case, handlers=None):
+        super(DjangoRunner, self).__init__(case, handlers=handlers)
+        self._django_case = self.MyDjangoTestCase('test_foo')
+
+    def _run_core(self):
+        self._run_user(self._django_case._pre_setup)
+        super(DjangoRunner, self)._run_core()
+        self._run_user(self._django_case._post_teardown)
+
+
+class EmptyBinaryPackageDatabase(Fixture):
+
+    def setUp(self):
+        super(EmptyBinaryPackageDatabase, self).setUp()
+        db_directory = self.useFixture(TempDir()).path
+        db_path = os.path.join(db_directory, 'test.db')
+        self.useFixture(
+            EnvironmentVariableFixture(PackageDatabase.ENV_VAR, db_path))
+
 
 class TestCaseWithFactory(TestCase):
     factory = PkgmeObjectFactory()
+    run_tests_with = DjangoRunner

=== modified file 'src/djpkgme/tests/test_handlers.py'
--- src/djpkgme/tests/test_handlers.py	2011-09-01 20:35:22 +0000
+++ src/djpkgme/tests/test_handlers.py	2011-11-22 14:09:24 +0000
@@ -78,16 +78,20 @@
 
 
     @patch('httplib2.Http.request')
-    def test_no_overrides_success(self, mock_request):
+    @patch('djpkgme.tasks.BuildPackageTask.build_package')
+    def test_no_overrides_success(self, mock_build_package, mock_request):
         request = Mock()
+        mock_build_package.return_value = 'foo'
         request.data = {'metadata': test_metadata, 'overrides': {}}
         handler = PackageHandler()
         response = handler.create(request)
         self.assertEqual({'status': 'success'}, response)
 
     @patch('httplib2.Http.request')
-    def test_overrides_success(self, mock_request):
+    @patch('djpkgme.tasks.BuildPackageTask.build_package')
+    def test_overrides_success(self, mock_build_package, mock_request):
         request = Mock()
+        mock_build_package.return_value = 'foo'
         request.data = {'metadata': test_metadata, 'overrides': {
             'executable': 'bin/foo',
             'deps': ['libfoo', 'libbar'],

=== modified file 'src/djpkgme/tests/test_models.py'
--- src/djpkgme/tests/test_models.py	2011-09-01 21:46:47 +0000
+++ src/djpkgme/tests/test_models.py	2011-11-22 14:09:24 +0000
@@ -1,6 +1,13 @@
-from factory import TestCaseWithFactory
+from djpkgme.tests.factory import TestCaseWithFactory
+
+from devportalbinary.binary import (
+    CATEGORIES,
+    PACKAGE_NAME,
+    TAGLINE,
+    )
 
 class AppMetadataTestCase(TestCaseWithFactory):
+
     def test_update_guesses_doesnt_mutate_argument(self):
         """Calling update_guesses doesn't modify the dict we pass in."""
         metadata = self.factory.make_metadata(with_guesses=True)
@@ -50,4 +57,13 @@
 
         expected = ['dependencies', 'pkgme_distro_arch_series',
             'pkgme_extra_data', 'packaged_app_url']
-        self.assertEquals(set(expected), set(info))
\ No newline at end of file
+        self.assertEquals(set(expected), set(info))
+
+    def test_as_dict(self):
+        metadata = self.factory.make_metadata()
+        expected = {
+            CATEGORIES: metadata.categories,
+            PACKAGE_NAME: metadata.package_name,
+            TAGLINE: metadata.tagline,
+            }
+        self.assertEqual(expected, metadata.as_dict())

=== modified file 'src/djpkgme/tests/test_tasks.py'
--- src/djpkgme/tests/test_tasks.py	2011-09-01 21:46:47 +0000
+++ src/djpkgme/tests/test_tasks.py	2011-11-22 14:09:24 +0000
@@ -1,15 +1,29 @@
-# Run tasks eagerly for tests.  Taken from 
+# Run tasks eagerly for tests.  Taken from
 # http://ask.github.com/celery/cookbook/unit-testing.html
 
 import json
+import os
+import shutil
+import tarfile
+
+from fixtures import TempDir
 from mock import patch
 
-from djpkgme.tasks import GuessPackagingDetailsTask, BuildPackageTask
-from djpkgme.tests.factory import TestCaseWithFactory
+from devportalbinary.binary import METADATA_FILE
+from djpkgme import tasks
+from djpkgme.tasks import (
+    BuildPackageTask,
+    GuessPackagingDetailsTask,
+    )
+from djpkgme.tests.factory import (
+    EmptyBinaryPackageDatabase,
+    TestCaseWithFactory,
+    )
+
 
 class GuessPackagingDetailsTaskTestCase(TestCaseWithFactory):
     @patch('httplib2.Http.request')
-    @patch('djpkgme.tasks.pkgme')
+    @patch('djpkgme.tasks.mock_pkgme')
     def test_guess_details_is_called(self, mock_pkgme, mock_request):
         mock_request.return_value = ({}, '')
         mock_pkgme.guess.return_value = {}
@@ -23,7 +37,7 @@
         self.assertEqual(1, mock_pkgme.guess.call_count)
 
     @patch('httplib2.Http.request')
-    @patch('djpkgme.tasks.pkgme')
+    @patch('djpkgme.tasks.mock_pkgme')
     def test_result_is_submitted(self, mock_pkgme, mock_request):
         mock_request.return_value = ({}, '')
         mock_pkgme.guess.return_value = {}
@@ -35,20 +49,21 @@
             "pkgme_extra_data": {"pkgme_id": 1, "executable": ""}
         }
 
-        result = GuessPackagingDetailsTask.delay(metadata)
+        GuessPackagingDetailsTask.delay(metadata)
 
         self.assertEqual(1, mock_request.call_count)
         args, kwargs = mock_request.call_args
-        self.assertEqual((metadata.callback_url,), args)
-        self.assertEqual(json.loads(kwargs['body']), expected)
-        
+        self.assertEqual(args, (metadata.callback_url,))
+        self.assertEqual(expected, json.loads(kwargs['body']))
+
 
 class BuildPackageTaskTestCase(TestCaseWithFactory):
+
     @patch('httplib2.Http.request')
-    @patch('djpkgme.tasks.pkgme')
-    def test_guess_details_is_called(self, mock_pkgme, mock_request):
+    @patch('djpkgme.tasks.BuildPackageTask.build_package')
+    def test_build_package_is_called(self, mock_build_package, mock_request):
         mock_request.return_value = ({}, '')
-        mock_pkgme.build_package.return_value = "foo"
+        mock_build_package.return_value = "foo"
 
         metadata = self.factory.make_metadata()
         overrides = {}
@@ -57,18 +72,18 @@
 
         self.assertEqual('foo', result.get())
         self.assertTrue(result.successful())
-        self.assertEqual(1, mock_pkgme.build_package.call_count)
+        self.assertEqual(1, mock_build_package.call_count)
 
     @patch('httplib2.Http.request')
-    @patch('djpkgme.tasks.pkgme')
-    def test_result_is_submitted(self, mock_pkgme, mock_request):
+    @patch('djpkgme.tasks.BuildPackageTask.build_package')
+    def test_result_is_submitted(self, mock_build_package, mock_request):
         mock_request.return_value = ({}, '')
-        mock_pkgme.build_package.return_value = "foo"
+        mock_build_package.return_value = "foo"
 
         metadata = self.factory.make_metadata()
         overrides = {}
 
-        result = BuildPackageTask.delay(metadata, overrides)
+        BuildPackageTask.delay(metadata, overrides)
 
         self.assertEqual(1, mock_request.call_count)
         args, kwargs = mock_request.call_args
@@ -78,5 +93,45 @@
             "pkgme_distro_arch_series": [],
             "pkgme_extra_data": {"pkgme_id": 1, "executable": ""}
         }
-        self.assertEqual((metadata.callback_url,), args)
-        self.assertEqual(json.loads(kwargs['body']), expected)
+        self.assertEqual(args, (metadata.callback_url,))
+        self.assertEqual(expected, json.loads(kwargs['body']))
+
+    def download_file(self, url, directory):
+        # For the moment, the test we just want to create a valid tarball in
+        # the directory.
+        import acceptance
+        binary_path = os.path.join(
+            os.path.dirname(acceptance.__file__), 'data', 'gtk', 'test_gtk')
+        temp_dir = self.useFixture(TempDir()).path
+        nothing_file_path = os.path.join(temp_dir, 'binary-file')
+        shutil.copyfile(binary_path, nothing_file_path)
+        tarball_path = os.path.join(directory, 'tarball.tar.gz')
+        tarball = tarfile.open(tarball_path, 'w')
+        tarball.add(nothing_file_path, os.path.basename(nothing_file_path))
+        tarball.close()
+        return tarball_path
+
+    def test_writes_packaging_for_tarball(self):
+        # BuildPackageTask.build_package runs pkgme to build the package and
+        # outputs a tarball to the given path. Here we're testing the happy
+        # case.
+        metadata = self.factory.make_metadata()
+        task = BuildPackageTask()
+        self.patch(tasks, 'download_file', self.download_file)
+        self.useFixture(EmptyBinaryPackageDatabase())
+        temp_dir = self.useFixture(TempDir()).path
+        output_tar_path = os.path.join(temp_dir, 'output.tar.gz')
+        task.build_package(metadata, {}, output_tar_path)
+        with tarfile.open(output_tar_path) as output_tar:
+            self.assertEqual(
+                sorted(
+                    ['%s_0.dsc' % (metadata.package_name,),
+                     '%s_0.tar.gz' % (metadata.package_name,),
+                     '%s_0_source.build' % (metadata.package_name,),
+                     '%s_0_source.changes' % (metadata.package_name,),
+                     METADATA_FILE,
+                     ]),
+                sorted(output_tar.getnames()))
+
+    # XXX: Write a test directly for the contents of METADATA_FILE?
+    # XXX: Check the contents of the tarball in the source package?

=== modified file 'test-dependencies.txt'
--- test-dependencies.txt	2011-11-09 11:14:13 +0000
+++ test-dependencies.txt	2011-11-22 14:09:24 +0000
@@ -1,4 +1,9 @@
+coverage
 django-kombu
-coverage
+mock
 piston-mini-client
-mock
+testtools
+# These aren't test requirements, but we want to fetch these dependencies from
+# bzr and this is the only way I can figure out how to do this -- jml
+-e bzr+ssh://bazaar.launchpad.net/+branch/pkgme#egg=pkgme
+-e bzr+ssh://bazaar.launchpad.net/+branch/pkgme-binary#egg=pkgme-binary