curtin-dev team mailing list archive

[Michael Hudson-Doyle <mp+466271@xxxxxxxxxxxxxxxxxx>]

Review: Approve

I wonder why jenkins hasn't reported on this yet?

Diff comments:

> diff --git a/curtin/config.py b/curtin/config.py
> index b30ecc5..979bb09 100644
> --- a/curtin/config.py
> +++ b/curtin/config.py
> @@ -154,6 +154,13 @@ class GrubConfig:
>      update_nvram: bool = attr.ib(default=True, converter=value_as_boolean)
>  
>  
> +@attr.s(auto_attribs=True)
> +class KernelConfig:
> +    package: typing.Optional[str] = None
> +    fallback_package: str = "linux-generic"
> +    mapping: dict = {}

= attr.Factory(dict) pls

> +
> +
>  class SerializationError(Exception):
>      def __init__(self, obj, path, message):
>          self.obj = obj
> diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
> index b73a19f..cbca425 100644
> --- a/tests/unittests/test_curthooks.py
> +++ b/tests/unittests/test_curthooks.py
> @@ -74,30 +79,51 @@ class TestCurthooksInstallKernel(CiTestCase):
>          with patch.dict(os.environ, clear=True):
>              curthooks.install_kernel(self.kernel_cfg, self.target)
>  
> -            env = {'FK_FORCE': 'yes', 'FK_FORCE_CONTAINER': 'yes'}
> -
>              inst_calls = [
>                  call(['u-boot-tools'], target=self.target),
> -                call([kernel_package], target=self.target, env=env)]
> +                call([kernel_package], target=self.target, env=self.fk_env)]
>  
>              self.mock_instpkg.assert_has_calls(inst_calls)
>  
>      def test__installs_kernel_package(self):
>          kernel_package = self.kernel_cfg.get('kernel', {}).get('package', {})
> -        self.mock_get_flash_kernel_pkgs.return_value = None
>  
>          with patch.dict(os.environ, clear=True):
>              curthooks.install_kernel(self.kernel_cfg, self.target)
>  
> -            env = {'FK_FORCE': 'yes', 'FK_FORCE_CONTAINER': 'yes'}
> +            self.mock_instpkg.assert_called_with(
> +                [kernel_package], target=self.target, env=self.fk_env)
> +
> +    def test__installs_kernel_fallback_package(self):
> +        del self.kernel_cfg['kernel']['package']

You don't need to fix it here but this pattern of having setUp() define self.kernel_cfg and each test mutating/referring to it as needed is a bit odd. Is there a reason a test can't just create the config it needes?

> +        fallback = self.kernel_cfg['kernel']['fallback-package']
> +
> +        self.mock_subp.return_value = ("warty", "")
> +        self.mock_uname.return_value = (None, None, "1.2.3-4-flavor")
> +
> +        with patch.dict(os.environ, clear=True):
> +            curthooks.install_kernel(self.kernel_cfg, self.target)
> +
> +            self.mock_instpkg.assert_called_with(
> +                [fallback], target=self.target, env=self.fk_env)
> +
> +    def test__installs_kernel_from_mapping(self):
> +        del self.kernel_cfg['kernel']['package']
> +        self.kernel_cfg['kernel']['mapping'] = {
> +            "warty": {"1.2.3": "-lts-dapper"}
> +        }
> +        self.mock_subp.return_value = ("warty", "")
> +        self.mock_uname.return_value = (None, None, "1.2.3-4-flavor")
> +
> +        with patch.dict(os.environ, clear=True):
> +            curthooks.install_kernel(self.kernel_cfg, self.target)
>  
>              self.mock_instpkg.assert_called_with(
> -                [kernel_package], target=self.target, env=env)
> +                ["linux-flavor-lts-dapper"],
> +                target=self.target, env=self.fk_env)
>  
>      def test__installs_kernel_null(self):
>          kernel_cfg = {'kernel': None}
> -        self.mock_get_flash_kernel_pkgs.return_value = None
> -
>          with patch.dict(os.environ, clear=True):
>              curthooks.install_kernel(kernel_cfg, self.target)
>  


-- 
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/466271
Your team curtin developers is subscribed to branch curtin:master.