I have to say I find the test_apt_source tests a bit incomprehensible. Your
changes don't really make them worse but I don't think they really make them
better either :/ I have a couple of concrete suggestions in the comments, do
they sound reasonable?
Diff comments:
> diff --git a/tests/unittests/test_apt_source.py
> b/tests/unittests/test_apt_source.py
> index 6556399..c89855f 100644
> --- a/tests/unittests/test_apt_source.py
> +++ b/tests/unittests/test_apt_source.py
> @@ -75,6 +90,9 @@ class TestAptSourceConfig(CiTestCase):
> self.aptlistfile3 = os.path.join(self.tmp, "single-deb3.list")
> self.join = os.path.join
> self.matcher = re.compile(ADD_APT_REPO_MATCH).search
> + # in case this is not obvious to anyone else, this causes GPG
> + # armored keys to be 's390x', because the underlying gpg code
> + # calls util.subp.
It seems like nothing depends on this mocking (any more?) so let's not make the
tests awful because of it :) Is it possible to change the tests so that it's
the functions from the gpg module that are mocked?
> self.add_patch('curtin.util.subp', 'm_subp')
> self.m_subp.return_value = ('s390x', '')
>
> @@ -221,17 +239,24 @@ class TestAptSourceConfig(CiTestCase):
> """
> params = self._get_default_params()
>
> - with mock.patch("curtin.util.subp",
> - return_value=('fakekey 1234', '')) as mockobj:
> - self._add_apt_sources(cfg, TARGET, template_params=params,
> - aa_repo_match=self.matcher)
> + self._add_apt_sources(cfg, TARGET, template_params=params,
> + aa_repo_match=self.matcher)
>
> - # check if it added the right ammount of keys
> - calls = []
> - for _ in range(keynum):
This method (_apt_src_keyid) no longer uses the keynum argument, so drop it?
> - calls.append(call(['apt-key', 'add', '-'], data=b'fakekey 1234',
> - target=TARGET))
> - mockobj.assert_has_calls(calls, any_order=True)
> + for ent in cfg:
> + key_filename = (
> + cfg[ent]['filename'] if 'filename' in cfg[ent] else ent
> + )
key_filename = cfg[ent].get('filename', ent) is clearer, surely?
> + # without knowing the key, do not know the suffix
> + self.assertTrue(
> + os.path.isfile(key_filename + '.asc') or
> + os.path.isfile(key_filename + '.gpg')
> + )
> + if os.path.isfile(key_filename + '.asc'):
> + contents = load_tfile(key_filename + '.asc')
> + self.assertMultiLineEqual(EXPECTEDKEY_NOVER, contents)
> + elif os.path.isfile(key_filename + '.gpg'):
> + contents = load_tfile(key_filename + '.gpg')
> + self.assertMultiLineEqual(EXPECTED_BINKEY, contents)
>
> self.assertTrue(os.path.isfile(filename))
>
--
https://code.launchpad.net/~nacc/curtin/+git/curtin/+merge/393661
Your team curtin developers is subscribed to branch curtin:master.
--
Mailing list: https://launchpad.net/~curtin-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~curtin-dev
More help : https://help.launchpad.net/ListHelp
