fix comments

Author: rsarm

reframe/core/containerplatform.py renamed to reframe/core/containers.py

@@ -2,7 +2,7 @@
 
 import reframe.core.fields as fields
 import reframe.utility.typecheck as typ
-from reframe.core.exceptions import ContainerPlatformError
+from reframe.core.exceptions import ContainerError
 
 
 class ContainerPlatform:
@@ -15,72 +15,83 @@ class ContainerPlatform:
     registry = fields.TypedField('registry', str, type(None))
     image = fields.TypedField('image', str, type(None))
     requires_mpi = fields.TypedField('requires_mpi', bool)
-    commands = fields.TypedField('commands', typ.List[str], type(None))
+    commands = fields.TypedField('commands', typ.List[str])
     mount_points = fields.TypedField('mount_points',
-                                     typ.List[typ.Tuple[str, str]], type(None))
+                                     typ.List[typ.Tuple[str, str]])
+    workdir = fields.TypedField('workdir', str, type(None))
 
     def __init__(self):
         self.registry = None
         self.image = None
         self.requires_mpi = False
-        self.commands = None
+        self.commands = []
         self.mount_points  = []
+        self.workdir = None
 
     @abc.abstractmethod
     def emit_prepare_cmds(self):
-        """xxx."""
+        """Returns commands that are necessary before running with this
+        container platform.
+
+        :raises: `ContainerError` in case of errors.
+
+        .. note:
+            This method is relevant only to developers of new container
+            platforms.
+        """
 
     @abc.abstractmethod
     def emit_launch_cmds(self):
         """Returns the command for running with this container platform.
 
-        :raises: `ContainerPlatformError` in case of missing mandatory
-        fields.
+        :raises: `ContainerError` in case of errors.
+
+        .. note:
+            This method is relevant only to developers of new container
+            platforms.
+        """
+        if self.registry:
+            self.image = '/'.join([self.registry, self.image])
+
+    @abc.abstractmethod
+    def validate(self):
+        """Validates this container platform.
+
+        :raises: `ContainerError` in case of errors.
 
         .. note:
             This method is relevant only to developers of new container
-            plataforms.
+            platforms.
         """
+        if self.image is None:
+            raise ContainerError('no image specified.')
+
+        if not self.commands:
+            raise ContainerError('no command specified')
 
 
 class Docker(ContainerPlatform):
-    """An implementation of the container platform to run containers with
+    """An implementation of ContainerPlatform to run containers with
     Docker.
     """
-    # def __init__(self):
-    #     super().__init__()
-
     def emit_prepare_cmds(self):
         pass
 
     def emit_launch_commands(self):
+        super().emit_launch_cmds()
         docker_opts = []
-
-        if self.image is None:
-            raise ContainerPlatformError('Please, specify the name of'
-                                         'the image')
-
-        if self.commands is None:
-            raise ContainerPlatformError('Please, specify a command')
-
-        if self.registry:
-            self.image = '%s/%s' % (self.registry, self.image)
-
-        self.mount_points.append(('$PWD', '/stagedir'))
-        for mp in self.mount_points:
-            docker_opts.append('-v %s:%s' % mp)
-
-        cmd_base = "docker run %s %s bash -c 'cd /stagedir; %s'"
-
-        return [cmd_base % (' '.join(docker_opts), self.image,
-                            '; '.join(self.commands))]
+        docker_opts = ['-v "%s":"%s"' % mp for mp in self.mount_points]
+        run_cmd = 'docker run %s %s bash -c ' % (' '.join(docker_opts),
+                                                 self.image)
+        return run_cmd + "'" + '; '.join(
+            ['cd ' + self.workdir] + self.commands) + "'"
 
 
 class ContainerPlatformField(fields.TypedField):
-    """A field representing a build system.
+    """A field representing a container platforms.
 
     You may either assign an instance of :class:`ContainerPlatform:` or a
-    string representing the name of the concrete class of a build system.
+    string representing the name of the concrete class of a container platform.
     """
 
     def __init__(self, fieldname, *other_types):

reframe/core/exceptions.py

@@ -108,7 +108,7 @@ class BuildSystemError(ReframeError):
     """Raised when a build system is not configured properly."""
 
 
-class ContainerPlatformError(ReframeError):
+class ContainerError(ReframeError):
     """Raised when a container platform is not configured properly."""
 
 

unittests/test_containerplatform.py

@@ -0,0 +1,68 @@
+import abc
+import unittest
+
+import pytest
+import reframe.core.containers as containers
+from reframe.core.exceptions import ContainerError
+
+
+class _ContainerPlatformTest:
+    @abc.abstractmethod
+    def create_container_platform(self):
+        pass
+
+    @property
+    @abc.abstractmethod
+    def exp_cmd_mount_points(self):
+        pass
+
+    @property
+    @abc.abstractmethod
+    def exp_cmd_custom_registry(self):
+        pass
+
+    def setUp(self):
+        self.container_platform = self.create_container_platform()
+
+    def test_mount_points(self):
+        self.container_platform.image = 'name:tag'
+        self.container_platform.mount_points = [('/path/one', '/one'),
+                                                ('/path/two', '/two')]
+        self.container_platform.commands = ['cmd1', 'cmd2']
+        self.container_platform.workdir = '/stagedir'
+        self.assertEqual(self.exp_cmd_mount_points,
+                         self.container_platform.emit_launch_commands())
+
+    def test_missing_image(self):
+        self.container_platform.commands = ['cmd']
+        with pytest.raises(ContainerError):
+            self.container_platform.validate()
+
+    def test_missing_commands(self):
+        self.container_platform.image = 'name:tag'
+        with pytest.raises(ContainerError):
+            self.container_platform.validate()
+
+    def test_custom_registry(self):
+        self.container_platform.registry = 'registry/custom'
+        self.container_platform.image = 'name:tag'
+        self.container_platform.commands = ['cmd']
+        self.container_platform.mount_points = [('/path/one', '/one')]
+        self.container_platform.workdir = '/stagedir'
+        self.assertEqual(self.exp_cmd_custom_registry,
+                         self.container_platform.emit_launch_commands())
+
+
+class TestDocker(_ContainerPlatformTest, unittest.TestCase):
+    def create_container_platform(self):
+        return containers.Docker()
+
+    @property
+    def exp_cmd_mount_points(self):
+        return ('docker run -v "/path/one":"/one" -v "/path/two":"/two" '
+                "name:tag bash -c 'cd /stagedir; cmd1; cmd2'")
+
+    @property
+    def exp_cmd_custom_registry(self):
+        return ('docker run -v "/path/one":"/one" registry/custom/name:tag '
+                "bash -c 'cd /stagedir; cmd'")