Improve branch docker generation

Dockerfile-branch.model

@@ -4,6 +4,6 @@ LABEL maintainer="PrestaShop Core Team <[email protected]>"
 RUN apt update
 RUN apt -y install git
 
-RUN git clone -b $ps_version https://github.com/PrestaShop/PrestaShop.git /tmp/data-ps
+RUN git clone -b $branch_version https://github.com/PrestaShop/PrestaShop.git /tmp/data-ps
 
 CMD ["/tmp/docker_run.sh"]

HOW-TO-USE.md

@@ -70,3 +70,44 @@ docker compose up generate
 ```
 
 This will create new folders for the new version you just added.
+
+## Running tests locally
+
+To run the python tests you need to install requirements
+
+```bash
+$ pip install -r requirements.txt
+```
+
+Then you can run the tests:
+
+```bash
+$ nosetests
+```
+
+Locally you may have an error like ``, running these commands may help running tests locally:
+
+```bash
+$ pip uninstall -y nose
+$ pip install -U nose --no-binary :all:
+```
+
+or alternatively:
+
+```bash
+$ pip install nose-py3
+```
+
+If you need to debug one specific test you first need to run
+
+```
+$ nosetests --with-id
+```
+
+This will execute tests and each test method will be assigned an ID that you can then use to filter it specifically:
+
+```
+$ nosetests --with-id 7
+```
+
+This will also generate a `.nodeids` binary file, when you add new test methods you need to remove this file to re-generate the list of IDs.

prestashop_docker/generator.py

@@ -3,6 +3,7 @@
 from string import Template
 from packaging import version
 from . import CONTAINERS
+from prestashop_docker.version_manager import VersionManager
 
 
 class Generator:
@@ -27,6 +28,7 @@ def __init__(self, directory_path, template, nightly_template, branch_template):
         self.template = Template(template)
         self.nightly_template = Template(nightly_template)
         self.branch_template = Template(branch_template)
+        self.version_manager = VersionManager(directory_path)
 
     def create_directory(self, directory_path):
         """Try to create a directory if it's possible
@@ -58,23 +60,35 @@ def generate_image(self, ps_version, container_version):
         self.create_directory(directory_path)
 
         file_path = path.join(directory_path, 'Dockerfile')
+        parsed_version = self.version_manager.get_version_from_string(ps_version)
+        split_version = self.version_manager.split_prestashop_version(ps_version)
+
         template = self.nightly_template if (
             ps_version == self.NIGHTLY
         ) else self.branch_template if (
-            ps_version.endswith('.x')
+            split_version is not None and split_version['patch'] == 'x'
         ) else self.template
 
+        # Get valid PS version (for branch versions it returns to future next patch)
+        ps_version = parsed_version['ps_version']
+        branch_version = parsed_version['branch_version']
+
         with open(file_path, 'w+') as f:
-            # We use 1.7.8.8 as the comparison base because the 1.7.8.9 is not hosted on the .com anymore but until 1.7.8.8 it still works,
-            # however we can't use 8.0 as the base because 8.0.0-beta is lower than 8.0 and we need beta versions of 8 to use the new url
-            if version.parse(ps_version) > version.parse('1.7.8.8'):
+            use_github_url = True
+            # We use 1.7.8.8 as the comparison base because the 1.7.8.9 is not hosted on the .com anymore but until 1.7.8.8,
+            # it still works so the .com url is used
+            if split_version is not None and split_version['major'] == '1.7' and version.parse(ps_version) <= version.parse('1.7.8.8'):
+                use_github_url = False
+
+            if use_github_url:
                 ps_url = self.download_url_github.format(ps_version, ps_version)
             else:
                 ps_url = self.download_url.format(ps_version)
             f.write(
                 template.substitute(
                     {
                         'ps_version': ps_version,
+                        'branch_version': branch_version,
                         'container_version': container_version,
                         'ps_url': ps_url
                     }

prestashop_docker/version_manager.py

@@ -83,12 +83,12 @@ def parse_version(self, version):
 
     def get_version_from_string(self, version):
         '''
-        Split version to find PrestaShop version, PHP version and container type
+        Split version to find PrestaShop version, branch version, PHP version and container type
 
         @param version: The version you want
         @type version: str
-        @return: A tuple containing ('PS_VERSION', (PHP_VERSIONS), 'CONTAINER_TYPE')
-                 or ('PS_VERSION', 'PHP_VERSION', 'CONTAINER_TYPE')
+        @return: A tuple containing ('PS_VERSION', 'BRANCH_VERSION', (PHP_VERSIONS), 'CONTAINER_TYPE')
+                 or ('PS_VERSION', 'BRANCH_VERSION', 'PHP_VERSION', 'CONTAINER_TYPE')
         @rtype: tuple
         '''
         matches = self.parse_version_from_string(version)
@@ -108,12 +108,83 @@ def get_version_from_string(self, version):
         if matches.group('container'):
             container_version = matches.group('container')
 
+        split_version = self.split_prestashop_version(ps_version)
+        if split_version is None:
+            branch_version = 'develop'
+        elif split_version['patch'] == 'x':
+            # ps_version actually already contains the branch
+            branch_version = ps_version
+            # We need to transform the branch into the next patch version
+            patch_index = ps_version.rindex('.')
+            last_patch = self.get_last_patch_from_version(ps_version)
+            if last_patch is None:
+                last_patch = '0'
+            else:
+                last_patch = str(int(last_patch) + 1)
+            ps_version = ps_version[:patch_index + 1] + last_patch
+        else:
+            # Transform the last patch version into an x to get the branch, we ignore any -rc that may be present
+            real_version = split_version['version']
+            patch_index = real_version.rindex('.')
+            branch_version = real_version[:patch_index + 1] + 'x'
+
         return {
             'ps_version': ps_version,
+            'branch_version': branch_version,
             'php_versions': php_versions,
             'container_version': container_version
         }
 
+    def get_last_patch_from_version(self, version):
+        '''
+        Get last patch version for the specified version based on the VERSIONS list
+        @param version: The version you need the match from
+        @type version: str
+        @return: Return None if no patch is found otherwise an int with the patch.
+        @rtpe: None|int
+        '''
+        split_version = self.split_prestashop_version(version)
+        if (split_version is None):
+            return None
+
+        lastPatch = None
+        for ps_version, php_versions in VERSIONS.items():
+            split_ps_version = self.split_prestashop_version(ps_version)
+            if split_ps_version is None:
+                continue
+            if (split_ps_version['major'] != split_version['major'] or split_ps_version['minor'] != split_version['minor']):
+                continue
+            if split_ps_version['patch'] == 'x':
+                continue
+            if (lastPatch is None or int(split_ps_version['patch']) > int(lastPatch)):
+                lastPatch = split_ps_version['patch']
+        return lastPatch
+
+    def split_prestashop_version(self, version):
+        '''
+        Split the version into major minor patch object, it is a custom-tailed alternative to semver.VersionInfo.parse
+        that can handle our development branches like 1.7.8.x, 8.0.x, ...
+        @param version: The version you need to split
+        @type version: str
+        @return: Return None if no patch is found otherwise an int with the patch.
+        @rtpe: None|tuple
+        '''
+        regex = r"^(?P<major>(1.)?[0-9]+)\.(?P<minor>[0-9]+)\.(?P<patch>[0-9x]+)(?P<prerelease>-(alpha|beta|rc)(?:\.\d+)?(?:\+\d+)?)?"
+        matches = re.search(regex, version)
+
+        if (matches and matches.group() and matches.group('major') and matches.group('major') and matches.group('major')):
+            # Remove the initial matched -
+            prerelease = matches.group('prerelease')[1:] if matches.group('prerelease') else None
+
+            return {
+                'version': matches.group('major') + '.' + matches.group('minor') + '.' + matches.group('patch'),
+                'major': matches.group('major'),
+                'minor': matches.group('minor'),
+                'patch': matches.group('patch'),
+                'prerelease': prerelease,
+            }
+        return None
+
     def parse_version_from_string(self, version):
         '''
         Parse version from string based on a regex
@@ -122,7 +193,7 @@ def parse_version_from_string(self, version):
         @return: Return None if no position in the string matches the pattern otherwise a Match object.
         @rtpe: None|Match
         '''
-        regex = r"^(?P<version>(?:[0-9]+\.){0,3}(?:[0-9]+|nightly)(?:-(?:alpha|beta|rc)(?:\.\d+)?(?:\+\d+)?)?)(?:-(?P<php>\d+\.\d+))?(?:-(?P<container>fpm|apache))?$"
+        regex = r"^(?P<version>(?:[0-9]+\.){0,3}(?:[0-9]+|nightly|x)(?:-(?:alpha|beta|rc)(?:\.\d+)?(?:\+\d+)?)?)(?:-(?P<php>\d+\.\d+))?(?:-(?P<container>fpm|apache))?$"
         return re.search(regex, version)
 
     def get_aliases(self):
@@ -150,7 +221,7 @@ def get_aliases(self):
                 if alias_version != 'latest':
                     self.append_to_aliases(aliases, ps_version, php_version, PREFERED_CONTAINER, alias_version + '-' + php_version)
 
-            # Check prefered container
+            # Check preferred container
             self.append_to_aliases(aliases, ps_version, alias_php_version, PREFERED_CONTAINER, alias_version)
 
             # Check containers
@@ -184,6 +255,14 @@ def get_ps_versions_aliases(self):
                 }
                 continue
 
+            # Ignore branch versions
+            split_version = self.split_prestashop_version(ps_version)
+            if split_version['patch'] == 'x':
+                aliases[ps_version] = {
+                    'value': ps_version
+                }
+                continue
+
             # PrestaShop versions before 8 are in format 1.MAJOR.MINOR.PATCH
             # Starting version 8, format is MAJOR.MINOR.PATCH
             splitted_version = ps_version.split('.', 1)

tests/prestashop_docker/test_generator.py

@@ -29,7 +29,7 @@ def setUp(self):
             contents='''
             CONTAINER_VERSION: $container_version
             RUN apt -y install git
-            RUN git clone -b $ps_version https://github.com/PrestaShop/PrestaShop.git /tmp/data-ps
+            RUN git clone -b $branch_version https://github.com/PrestaShop/PrestaShop.git /tmp/data-ps
             '''
         )
 
@@ -46,10 +46,10 @@ def test_create_directory(self):
         self.assertTrue(path.exists('/tmp/images/test'))
 
     def test_generate_image(self):
-        dockerfile = '/tmp/images/1.7.8/7.4-alpine/Dockerfile'
+        dockerfile = '/tmp/images/1.7.8.0/7.4-alpine/Dockerfile'
         self.assertFalse(path.exists(dockerfile))
         self.generator.generate_image(
-            '1.7.8',
+            '1.7.8.0',
             '7.4-alpine'
         )
         self.assertTrue(path.exists(dockerfile))
@@ -58,29 +58,67 @@ def test_generate_image(self):
             content = f.read()
             self.assertIn(
                 'PS_URL: https://www.prestashop.com/download/old/'
-                'prestashop_1.7.8.zip',
+                'prestashop_1.7.8.0.zip',
                 content
             )
-            self.assertIn('PS_VERSION: 1.7.8', content)
+            self.assertIn('PS_VERSION: 1.7.8.0', content)
+            self.assertIn('CONTAINER_VERSION: 7.4-alpine', content)
+
+    def test_generate_image_1788(self):
+        dockerfile = '/tmp/images/1.7.8.8/7.4-alpine/Dockerfile'
+        self.assertFalse(path.exists(dockerfile))
+        self.generator.generate_image(
+            '1.7.8.8',
+            '7.4-alpine'
+        )
+        self.assertTrue(path.exists(dockerfile))
+
+        with open(dockerfile) as f:
+            content = f.read()
+            self.assertIn(
+                'PS_URL: https://www.prestashop.com/download/old/'
+                'prestashop_1.7.8.8.zip',
+                content
+            )
+            self.assertIn('PS_VERSION: 1.7.8.8', content)
+            self.assertIn('CONTAINER_VERSION: 7.4-alpine', content)
+
+    def test_generate_image_1789(self):
+        dockerfile = '/tmp/images/1.7.8.9/7.4-alpine/Dockerfile'
+        self.assertFalse(path.exists(dockerfile))
+        self.generator.generate_image(
+            '1.7.8.9',
+            '7.4-alpine'
+        )
+        self.assertTrue(path.exists(dockerfile))
+
+        with open(dockerfile) as f:
+            content = f.read()
+            self.assertIn(
+                'PS_URL: https://github.com/PrestaShop/PrestaShop/releases/download/1.7.8.9/'
+                'prestashop_1.7.8.9.zip',
+                content
+            )
+            self.assertIn('PS_VERSION: 1.7.8.9', content)
             self.assertIn('CONTAINER_VERSION: 7.4-alpine', content)
 
     def test_generate_image_80(self):
-        dockerfile = '/tmp/images/8.0/7.4-alpine/Dockerfile'
+        dockerfile = '/tmp/images/8.0.0/7.4-alpine/Dockerfile'
         self.assertFalse(path.exists(dockerfile))
         self.generator.generate_image(
-            '8.0',
+            '8.0.0',
             '7.4-alpine'
         )
         self.assertTrue(path.exists(dockerfile))
 
         with open(dockerfile) as f:
             content = f.read()
             self.assertIn(
-                'PS_URL: https://github.com/PrestaShop/PrestaShop/releases/download/8.0/'
-                'prestashop_8.0.zip',
+                'PS_URL: https://github.com/PrestaShop/PrestaShop/releases/download/8.0.0/'
+                'prestashop_8.0.0.zip',
                 content
             )
-            self.assertIn('PS_VERSION: 8.0', content)
+            self.assertIn('PS_VERSION: 8.0.0', content)
             self.assertIn('CONTAINER_VERSION: 7.4-alpine', content)
 
     def test_generate_nightly_image(self):
@@ -116,29 +154,42 @@ def test_generate_branch_image(self):
                 'PS_URL',
                 content
             )
-            self.assertNotIn('PS_VERSION', content)
             self.assertIn('CONTAINER_VERSION: 8.1-alpine', content)
             self.assertIn('RUN apt -y install git', content)
             self.assertIn('RUN git clone -b 9.0.x https://github.com/PrestaShop/PrestaShop.git /tmp/data-ps', content)
 
     def test_generate_all(self):
         files = (
-            '/tmp/images/7.0/7.3-apache/Dockerfile',
-            '/tmp/images/7.0/7.3-fpm/Dockerfile',
-            '/tmp/images/7.0/7.2-apache/Dockerfile',
-            '/tmp/images/7.0/7.2-fpm/Dockerfile',
-            '/tmp/images/8.0/7.1-apache/Dockerfile',
-            '/tmp/images/8.0/7.1-fpm/Dockerfile',
-            '/tmp/images/8.0/5.6-apache/Dockerfile',
-            '/tmp/images/8.0/5.6-fpm/Dockerfile',
+            '/tmp/images/1.7.8.8/7.3-apache/Dockerfile',
+            '/tmp/images/1.7.8.8/7.3-fpm/Dockerfile',
+            '/tmp/images/1.7.8.8/7.2-apache/Dockerfile',
+            '/tmp/images/1.7.8.8/7.2-fpm/Dockerfile',
+            '/tmp/images/8.0.0/7.2-apache/Dockerfile',
+            '/tmp/images/8.0.0/7.2-fpm/Dockerfile',
+            '/tmp/images/8.0.0/8.1-apache/Dockerfile',
+            '/tmp/images/8.0.0/8.1-fpm/Dockerfile',
+            '/tmp/images/9.0.x/8.1-apache/Dockerfile',
+            '/tmp/images/9.0.x/8.1-fpm/Dockerfile',
+            '/tmp/images/9.0.x/8.2-apache/Dockerfile',
+            '/tmp/images/9.0.x/8.2-fpm/Dockerfile',
+            '/tmp/images/9.0.x/8.3-apache/Dockerfile',
+            '/tmp/images/9.0.x/8.3-fpm/Dockerfile',
+            '/tmp/images/nightly/8.1-apache/Dockerfile',
+            '/tmp/images/nightly/8.1-fpm/Dockerfile',
+            '/tmp/images/nightly/8.2-apache/Dockerfile',
+            '/tmp/images/nightly/8.2-fpm/Dockerfile',
+            '/tmp/images/nightly/8.3-apache/Dockerfile',
+            '/tmp/images/nightly/8.3-fpm/Dockerfile',
         )
         for f in files:
             self.assertFalse(path.exists(f))
 
         self.generator.generate_all(
             {
-                '7.0': ('7.2', '7.3'),
-                '8.0': ('7.1', '5.6'),
+                '1.7.8.8': ('7.2', '7.3'),
+                '8.0.0': ('7.2', '8.1'),
+                '9.0.x': ('8.1', '8.2', '8.3'),
+                'nightly': ('8.1', '8.2', '8.3'),
             }
         )