launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/lazr-postgresql/no-filename-sort into lp:lazr-postgresql.

Commit message:
Sort patches by their version elements, not by filename.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/lazr-postgresql/no-filename-sort/+merge/323008

Sort patches by their version elements, not by filename.

LP avoided the problem by zero-prefixing.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/lazr-postgresql/no-filename-sort into lp:lazr-postgresql.

=== modified file 'src/lazr/postgresql/tests/test_upgrade.py'
--- src/lazr/postgresql/tests/test_upgrade.py	2016-08-03 02:24:44 +0000
+++ src/lazr/postgresql/tests/test_upgrade.py	2017-04-23 23:47:11 +0000
@@ -458,6 +458,37 @@
                 CannotApply, AfterPreprocessing(lambda x:x.args[0], StartsWith(
                 "First patch has a type not listed in patch_types")))))
 
+    def test_sorting(self):
+        # Patches are sorted element-wise as ints, not by filename. This
+        # allows leading zeroes to be omitted.
+        patchdir = self.useFixture(TempDir()).path
+        con = psycopg2.connect(host=self.db.host, database=self.db.database)
+        self.addCleanup(con.close)
+        apply_patches_normal(con, patchdir)
+        patches = [
+            os.path.join(patchdir, patch)
+            for patch in [
+                'patch-2-2-2-std.sql',
+                'patch-2-2-3-std.sql',
+                'patch-2-2-22-std.sql',
+                'patch-2-10-0-std.sql',
+                'patch-2-11-0-std.sql',
+                'patch-10-0-0-std.sql',
+                ]
+            ]
+        for patch in patches:
+            with open(patch, 'wb'):
+                pass
+        expected = [
+            ((2, 2, 2), patches[0], PATCH_STANDARD),
+            ((2, 2, 3), patches[1], PATCH_STANDARD),
+            ((2, 2, 22), patches[2], PATCH_STANDARD),
+            ((2, 10, 0), patches[3], PATCH_STANDARD),
+            ((2, 11, 0), patches[4], PATCH_STANDARD),
+            ((10, 0, 0), patches[5], PATCH_STANDARD),
+            ]
+        self.assertEqual(expected, missing_patches(con, patchdir))
+
 
 class TestBranchInfo(ResourcedTestCase, TestCaseInTempDir):
 

=== modified file 'src/lazr/postgresql/upgrade.py'
--- src/lazr/postgresql/upgrade.py	2016-08-03 02:24:44 +0000
+++ src/lazr/postgresql/upgrade.py	2017-04-23 23:47:11 +0000
@@ -271,13 +271,15 @@
     all_patch_names = sorted(glob.glob(
         os.path.join(patchdir, 'patch-*-*-*-*.sql')))
     for patch_name in all_patch_names:
-        m = re.search('patch-(\d+)-(\d+)-(\d)-(std|direct|concurrent).sql$',
+        m = re.search('patch-(\d+)-(\d+)-(\d+)-(std|direct|concurrent).sql$',
             patch_name)
         major, minor, patch = [int(i) for i in m.groups()[:3]]
         patch_type = m.groups()[3]
         patch_info = (major, minor, patch)
         if patch_info not in existing_patches:
             found_patches.append((patch_info, patch_name, patch_type))
+    # Ensure they're sorted element-wise, not as raw filenames.
+    found_patches.sort()
     # Now post-process to only return one type of patch.
     group_type = None
     patches = []