Merge lp:~brian-murray/cupstream2distro/bug-1541258 into lp:cupstream2distro

[Brian Murray]

I wasn't sure how to make test_manager_get_published_versions work without having get_published_versions still return the versions dict.

[Robert Bruce Park (robru)]

I like the direction this is going in, just a couple comments inline.

review: Needs Fixing

[Robert Bruce Park (robru)]

On second thought, instead of calling the method from the end of get_states, consider merging the method into get_states directly. No point iterating over the builds twice.

[lp:~brian-murray/cupstream2distro/bug-1541258]

1356. By Brian Murray on 2016-02-03

address some feedback from robru

1357. By Brian Murray on 2016-02-03

move get_published_versions info into get_states

[Robert Bruce Park (robru)]

Great, thanks!

review: Approve

=== modified file 'citrain/recipes/base.py'
--- citrain/recipes/base.py	2016-01-15 21:31:00 +0000
+++ citrain/recipes/base.py	2016-02-03 23:29:24 +0000
@@ -44,12 +44,10 @@
 )
 from cupstream2distro.utils import (
     SILO_DIR,
-    atomic_write,
     env,
     log_value_of,
     name_ppa,
     suppress,
-    utf8_open,
 )
 
 
@@ -134,6 +132,7 @@
     our_version = None
     packagelist = None
     authorized = None
+    published = None
     failures = set()
 
     def __init__(self, source_name, series, dest, silo_ppa, primary=None):
@@ -232,23 +231,6 @@
         """Identify where keyfiles are located."""
         return '{}_{}'.format(self.path, key)
 
-    def set_file(self, key, value):
-        """Atomically write a value to a file."""
-        filename = self.find_file(key)
-        log_value_of.value('Writing')
-        log_value_of.filename('To')
-        with atomic_write(filename, 'w') as data:
-            data.write(str(value))
-
-    def get_file(self, key):
-        """Read value from disk.
-
-        :param key: Fragment of filename to read from.
-        """
-        with suppress(FileNotFoundError):
-            with utf8_open(self.find_file(key)) as data:
-                return data.read().strip()
-
     def validate_phase(self):
         """Stub validate phase."""
         pass

=== modified file 'citrain/recipes/manager.py'
--- citrain/recipes/manager.py	2016-01-22 02:33:07 +0000
+++ citrain/recipes/manager.py	2016-02-03 23:29:24 +0000
@@ -18,6 +18,7 @@
 These classes manage the instantiation of recipes and execution of phases.
 """
 
+import json
 import logging
 
 from collections import defaultdict
@@ -110,8 +111,13 @@
         """Report the statuses of all packages in this silo."""
         self.do()  # Instantiates build objects if necessary.
         states = defaultdict(list)
+        versions = defaultdict(dict)
         for name, build in sorted(self.builds.items()):
             states[build.check_status()].append(
                 '{}/{}'.format(build.name, build.series.name))
+            if not build.published:
+                continue
+            versions[build.name][build.series.name] = build.published
+        self.silo_state.published_versions = json.dumps(versions)
         log_value_of.states()
         return pretty_print_states(states)

=== modified file 'cupstream2distro/archive.py'
--- cupstream2distro/archive.py	2016-01-10 10:12:24 +0000
+++ cupstream2distro/archive.py	2016-02-03 23:29:24 +0000
@@ -229,7 +229,7 @@
                 version=version)
             if dest:
                 if self.archive_id(source) == self.archive_id(dest):
-                    self.set_file('published', version)
+                    self.published = version
                     return dest.pocket + ' pocket'
                 return 'Needs rebuild due to burned version number'
 

=== modified file 'cupstream2distro/silomanager.py'
--- cupstream2distro/silomanager.py	2016-02-02 22:55:33 +0000
+++ cupstream2distro/silomanager.py	2016-02-03 23:29:24 +0000
@@ -27,7 +27,7 @@
 import sys
 
 import os
-from os.path import basename, dirname, join
+from os.path import dirname, join
 
 from glob import glob
 from random import shuffle
@@ -354,19 +354,6 @@
             self.push_to_ppa_description()
 
     @property
-    def published_versions(self):
-        """Return a dictionary of what versions have been published."""
-        versions = defaultdict(dict)
-        fileglob = join(SILOS_DIR, self.siloname, '*', '*', '*published')
-        for filename in sorted(glob(fileglob)):
-            source = basename(filename).split('_')[0].split('+')[0]
-            series = filename.split('/')[-3]
-            with utf8_open(filename) as data:
-                version = data.read().strip()
-            versions[source][series] = version
-        return json.dumps(versions, sort_keys=True)
-
-    @property
     def all_projects(self):
         """Get a list of all projects."""
         return sorted(set(self.mps) | set(self.sources))

=== modified file 'tests/unit/test_recipe_base.py'
--- tests/unit/test_recipe_base.py	2016-01-15 21:31:00 +0000
+++ tests/unit/test_recipe_base.py	2016-02-03 23:29:24 +0000
@@ -95,7 +95,7 @@
         makedirs(build.path, exist_ok=True)
         build.get_package_version = Mock(return_value='42-0ubuntu1')
         self.assertEqual(build.check_status(), 'Proposed pocket')
-        self.assertEqual(build.get_file('published'), '42-0ubuntu1')
+        self.assertEqual(build.published, '42-0ubuntu1')
 
     @patch('cupstream2distro.archive.newest')
     def test_archive_pocket_mismatch(self, new):
@@ -211,14 +211,6 @@
         build2.buildstate = 'Successfully built'
         self.assertEqual(build.check_status(), 'Diff missing')
 
-    def test_get_file(self):
-        """Ensure we can read files from disk."""
-        build = BuildBase('getattr', self.series, self.dest, self.ppa)
-        makedirs(build.path, exist_ok=True)
-        build.set_file('published', 'hello')
-        self.assertEqual(build.get_file('published'), 'hello')
-        self.assertIsNone(build.get_file('nonexistent'))
-
     def test_pre_validate(self):
         """Test some nops."""
         mock = Mock()

=== modified file 'tests/unit/test_recipe_manager.py'
--- tests/unit/test_recipe_manager.py	2016-01-22 02:33:07 +0000
+++ tests/unit/test_recipe_manager.py	2016-02-03 23:29:24 +0000
@@ -15,6 +15,8 @@
 
 """Tests for CI Train Recipe Manager."""
 
+import json
+
 from mock import Mock, call, patch
 
 from tests.unit import DirectoryAwareTestCase
@@ -165,8 +167,10 @@
         silo_state = Mock()
         one = Mock(check_status=Mock(return_value='Successfully built'))
         one.name = 'one'
+        one.published = '1'
         two = Mock(check_status=Mock(return_value='Successfully built'))
         two.name = 'two'
+        two.published = '2'
         one.series.name = two.series.name = 'xenial'
         bman = Manager(silo_state)
         bman.types = True
@@ -178,8 +182,10 @@
         silo_state = Mock()
         one = Mock(check_status=Mock(return_value='Successfully built'))
         one.name = 'one'
+        one.published = '1'
         two = Mock(check_status=Mock(return_value='Failed to build'))
         two.name = 'two'
+        two.published = '2'
         one.series.name = two.series.name = 'xenial'
         bman = Manager(silo_state)
         bman.types = True
@@ -187,3 +193,23 @@
         self.assertEqual(
             bman.get_states(),
             'Failed to build (two/xenial). Successfully built (one/xenial).')
+
+    def test_manager_published_versions(self):
+        """Fetch the series and versions from a build."""
+        silo_state = Mock()
+        vivid = Mock(check_status=Mock(return_value='Successfully built'))
+        vivid.name = 'foo'
+        vivid.series.name = 'vivid'
+        vivid.published = '1'
+        xenial = Mock(check_status=Mock(return_value='Failed to build'))
+        xenial.name = 'bar'
+        xenial.series.name = 'xenial'
+        xenial.published = None
+        bman = Manager(silo_state)
+        bman.types = True
+        bman.builds = dict(one=vivid, two=xenial)
+        bman.get_states()
+        versions = {'foo': {'vivid': '1'}}
+        self.assertEqual(
+            silo_state.published_versions,
+            json.dumps(versions))

=== modified file 'tests/unit/test_silomanager.py'
--- tests/unit/test_silomanager.py	2016-02-02 22:09:56 +0000
+++ tests/unit/test_silomanager.py	2016-02-03 23:29:24 +0000
@@ -20,7 +20,7 @@
 import signal
 import logging
 
-from os.path import dirname, join
+from os.path import join
 from os import makedirs
 
 from mock import Mock, patch, call
@@ -29,7 +29,7 @@
 
 from cupstream2distro import silomanager
 from cupstream2distro.silomanager import SiloState, splitter, stock_main
-from cupstream2distro.utils import env, utf8_open
+from cupstream2distro.utils import env
 from cupstream2distro.errors import PrepError, NoStatusError
 
 
@@ -608,25 +608,6 @@
         self.assertEqual(self.state.dest, lp_mock.get_ppa.return_value)
         lp_mock.get_ppa.assert_called_once_with('team/ubuntu/ppa')
 
-    def test_published_versions(self):
-        """Return a json string representing the published versions."""
-        expected = dict(
-            alpha=['1.0+15.04', '1.0+16.04'],
-            beta=['1.0+15.04', '1.0+16.04'],
-        )
-        silodir = join(self.tempdir, self.state.siloname)
-        for source, versions in expected.items():
-            for version in versions:
-                series = 'vivid' if '15.04' in version else 'xenial'
-                path = join(silodir, series, source, source + '_published')
-                makedirs(dirname(path), exist_ok=True)
-                with utf8_open(path, 'w') as data:
-                    data.write(version)
-        self.assertEqual(
-            self.state.published_versions,
-            '{"alpha": {"vivid": "1.0+15.04", "xenial": "1.0+16.04"}, '
-            '"beta": {"vivid": "1.0+15.04", "xenial": "1.0+16.04"}}')
-
     @patch('cupstream2distro.silomanager.SiloState.sources', ['foobar'])
     @patch('cupstream2distro.silomanager.SiloState.mps', dict(merger=[]))
     def test_silostate_all_projects(self):