Hi Christian, On Tue, 4 Jul 2023 at 10:03, <christian.taedcke-...@weidmueller.com> wrote: > > From: Christian Taedcke <christian.taed...@weidmueller.com> > > Add tests to reach 100% code coverage for the added etype encrypted. > > Signed-off-by: Christian Taedcke <christian.taed...@weidmueller.com> > --- > > Changes in v2: > - adapt tests for changed entry implementation > > tools/binman/ftest.py | 52 +++++++++++++++++++ > .../binman/test/282_encrypted_no_content.dts | 15 ++++++ > tools/binman/test/283_encrypted_no_algo.dts | 19 +++++++ > .../test/284_encrypted_invalid_iv_file.dts | 23 ++++++++ > .../binman/test/285_encrypted_missing_key.dts | 28 ++++++++++ > .../binman/test/286_encrypted_key_source.dts | 29 +++++++++++ > tools/binman/test/287_encrypted_key_file.dts | 29 +++++++++++ > 7 files changed, 195 insertions(+) > create mode 100644 tools/binman/test/282_encrypted_no_content.dts > create mode 100644 tools/binman/test/283_encrypted_no_algo.dts > create mode 100644 tools/binman/test/284_encrypted_invalid_iv_file.dts > create mode 100644 tools/binman/test/285_encrypted_missing_key.dts > create mode 100644 tools/binman/test/286_encrypted_key_source.dts > create mode 100644 tools/binman/test/287_encrypted_key_file.dts > > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py > index 43b4f850a6..d51139799f 100644 > --- a/tools/binman/ftest.py > +++ b/tools/binman/ftest.py > @@ -94,6 +94,8 @@ ROCKCHIP_TPL_DATA = b'rockchip-tpl' > TEST_FDT1_DATA = b'fdt1' > TEST_FDT2_DATA = b'test-fdt2' > ENV_DATA = b'var1=1\nvar2="2"' > +ENCRYPTED_IV_DATA = b'123456' > +ENCRYPTED_KEY_DATA = b'1234567890123456'
Can you make that different, e.g. abcd, since at present one is a subset of the other. Does the length matter? If not, shorter is better for the second one, so we can visually look at the output. > PRE_LOAD_MAGIC = b'UBSH' > PRE_LOAD_VERSION = 0x11223344.to_bytes(4, 'big') > PRE_LOAD_HDR_SIZE = 0x00001000.to_bytes(4, 'big') > @@ -226,6 +228,10 @@ class TestFunctional(unittest.TestCase): > # Newer OP_TEE file in v1 binary format > cls.make_tee_bin('tee.bin') > > + # test files for encrypted tests > + TestFunctional._MakeInputFile('encrypted-file.iv', ENCRYPTED_IV_DATA) > + TestFunctional._MakeInputFile('encrypted-file.key', > ENCRYPTED_KEY_DATA) > + > cls.comp_bintools = {} > for name in COMP_BINTOOLS: > cls.comp_bintools[name] = bintool.Bintool.create(name) > @@ -6676,6 +6682,52 @@ fdt fdtmap Extract the > devicetree blob from the fdtmap > ['fit']) > self.assertIn("Node '/fit': Missing tool: 'mkimage'", > str(e.exception)) > > + def testEncryptedNoContent(self): """Test for missing content property""" Please add a comment to following test functions > + with self.assertRaises(ValueError) as e: > + self._DoReadFileDtb('282_encrypted_no_content.dts') > + self.assertIn("Node '/binman/fit/images/u-boot/encrypted': > Collection must have a 'content' property", str(e.exception)) Please wrap to 80cols. It is OK not to split strings though, but otherwise, please keep to 80. self.assertIn( "Node '/binman/fit/images/u-boot/encrypted': Collection must have a 'content' property", str(e.exception)) same below > + > + def testEncryptedNoAlgo(self): > + with self.assertRaises(ValueError) as e: > + self._DoReadFileDtb('283_encrypted_no_algo.dts') > + self.assertIn("Node '/binman/fit/images/u-boot/encrypted': > 'encrypted' entry is missing properties: algo iv-filename", str(e.exception)) > + > + def testEncryptedInvalidIvfile(self): > + with self.assertRaises(ValueError) as e: > + self._DoReadFileDtb('284_encrypted_invalid_iv_file.dts') > + self.assertIn("Filename 'invalid-iv-file' not found in input path", > + str(e.exception)) > + > + def testEncryptedMissingKey(self): > + with self.assertRaises(ValueError) as e: > + self._DoReadFileDtb('285_encrypted_missing_key.dts') > + self.assertIn("Node '/binman/fit/images/u-boot/encrypted': Provide > either 'key-filename' or 'key-source'", > + str(e.exception)) > + > + def testEncryptedKeySource(self): > + data = self._DoReadFileDtb('286_encrypted_key_source.dts')[0] > + > + dtb = fdt.Fdt.FromData(data) > + dtb.Scan() > + > + node = dtb.GetNode('/images/u-boot/cipher') > + self.assertEqual('algo-name', node.props['algo'].value) > + self.assertEqual('key-source-value', node.props['key-source'].value) > + self.assertEqual(ENCRYPTED_IV_DATA, > tools.to_bytes(''.join(node.props['iv'].value))) > + self.assertNotIn('key', node.props) > + > + def testEncryptedKeyFile(self): > + data = self._DoReadFileDtb('287_encrypted_key_file.dts')[0] > + > + dtb = fdt.Fdt.FromData(data) > + dtb.Scan() > + > + node = dtb.GetNode('/images/u-boot/cipher') > + self.assertEqual('algo-name', node.props['algo'].value) > + self.assertEqual(ENCRYPTED_IV_DATA, > tools.to_bytes(''.join(node.props['iv'].value))) > + self.assertEqual(ENCRYPTED_KEY_DATA, > b''.join(node.props['key'].value)) > + self.assertNotIn('key-source', node.props) > + > > if __name__ == "__main__": > unittest.main() > diff --git a/tools/binman/test/282_encrypted_no_content.dts > b/tools/binman/test/282_encrypted_no_content.dts > new file mode 100644 > index 0000000000..03f7ffee90 > --- /dev/null > +++ b/tools/binman/test/282_encrypted_no_content.dts > @@ -0,0 +1,15 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/dts-v1/; > + > +/ { > + binman { > + fit { > + images { > + u-boot { > + encrypted { > + }; > + }; > + }; > + }; > + }; > +}; Do these need to be in a FIT? It is OK to do this, but I wonder if it is necessary for the purposes of your tests? Regards, Simon