curtin-dev team mailing list archive

[Michael Hudson-Doyle <mwhudsonlp@xxxxxxxxxxx>]

Just one question, but this looks good! Sad but not very surprised that this doesn't fix the bug.

Diff comments:

> diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
> index ca0fc90..532a00d 100644
> --- a/tests/unittests/test_curthooks.py
> +++ b/tests/unittests/test_curthooks.py
> @@ -543,9 +543,14 @@ class TestSetupZipl(CiTestCase):
>  
>      @patch('curtin.block.get_devices_for_mp')
>      @patch('platform.machine')
> -    def test_setup_zipl_writes_etc_zipl_conf(self, m_machine, m_get_devices):
> +    @patch('curtin.commands.block_meta.get_volume_spec')
> +    def test_setup_zipl_writes_etc_zipl_conf(
> +            self, m_get_volume_spec, m_machine, m_get_devices):
>          m_machine.return_value = 's390x'
>          m_get_devices.return_value = ['/dev/mapper/ubuntu--vg-root']
> +        m_get_volume_spec.return_value = \
> +            '/dev/disk/by-id/dm-uuid-LVM-'\

I'm not going to insist on this at all, but what do you think about setting the return_value to a random string (there is a .random_string() helper method) and then testing the 'root=' + this random value are in the output?

> +            'HNzpRb1VyhlJwt3nlvXsJH5KTpBUl2LpN08GkOLhEwDmN4an0K13rMAg7eJxxeBT'
>          curthooks.setup_zipl(None, self.target)
>          m_get_devices.assert_called_with(self.target)
>          with open(os.path.join(self.target, 'etc', 'zipl.conf')) as stream:


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