Hi Simon,
On 8/31/22 15:46, Simon Glass wrote:
Hi Quentin,
On Wed, 31 Aug 2022 at 03:25, Quentin Schulz quentin.schulz@theobroma-systems.com wrote:
Hi Simon,
On 8/31/22 05:15, Simon Glass wrote:
Hi Quentin,
On Tue, 30 Aug 2022 at 11:54, Quentin Schulz quentin.schulz@theobroma-systems.com wrote:
Hi Simon,
On 8/30/22 17:56, Simon Glass wrote:
Hi Quentin,
On Tue, 30 Aug 2022 at 03:57, Quentin Schulz quentin.schulz@theobroma-systems.com wrote:
[...]
I do not understand how you found out coverage was not happy about my patchset. I have the same percentage reported from your branch or my local one. What am I missing?
Regarding the content of the changed commits: testMkimageMultipleNoContent is not testing what is says it does? It's using multiple-data-files DT property which only impacts -d parameter of mkimage and the comment for the test is """Test using mkimage with -n and no data""".
What exactly are you trying to test?
'binman test -T'
I pushed your original patches to the try-rk4-orig branch. My changes are in try-rk4.
I would change https://github.com/sjg20/u-boot/blob/try-rk4/tools/binman/ftest.py#L5917 from """Test using mkimage with -n and no data""" to """Test passing multiple data files to mkimage with one data file having no content""" or something like that. Do you want me to send a v6 for this?
Otherwise, looks fine to me :)
With the bzip2 patch I just sent, I could see the line missing from unit tests.
However, unit tests aren't happy on my PC.
$ tools/binman/binman tool --list Name Version Description Path --------------- ----------- ------------------------- ------------------------------ gzip 1.11 gzip compression /usr/bin/gzip bzip2 1.0.8 bzip2 compression /usr/bin/bzip2 cbfstool unknown Manipulate CBFS files /home/qschulz/work/upstream/coreboot/util/cbfstool/cbfstool fiptool Unknown vers Manipulate ATF FIP files /home/qschulz/work/upstream/trusted-firmware-a/tools/fiptool/fiptool futility v0.0.3050-18 Chromium OS firmware utili /home/qschulz/work/upstream/coreboot/util/futility/futility ifwitool unknown Manipulate Intel IFWI file /home/qschulz/work/upstream/coreboot/util/cbfstool/ifwitool lz4 v1.9.3 lz4 compression /usr/bin/lz4 lzma_alone - lzma_alone compression (not found) lzop v1.04 lzo compression /usr/bin/lzop mkimage 2022.10-rc3- Generate image for U-Boot /usr/bin/mkimage xz 5.2.5 xz compression /usr/bin/xz zstd v1.5.2 zstd compression /usr/bin/zstd $ tools/binman/binman test -T ======================== Running binman tests ======================== .........................................................sss.............................................F....Fs..............................................................................................F.................................................................................................................................................EEEEFF.................................FF.......FF............F................F.FFFF................. ====================================================================== ERROR: testSymbols (binman.ftest.TestFunctional) Test binman can assign symbols embedded in U-Boot ---------------------------------------------------------------------- KeyError: '_binman_sym_magic'
====================================================================== ERROR: testSymbolsExpanded (binman.ftest.TestFunctional) Test binman can assign symbols in expanded entries ---------------------------------------------------------------------- KeyError: '_binman_sym_magic'
====================================================================== ERROR: testSymbolsNoDtb (binman.ftest.TestFunctional) Test binman can assign symbols embedded in U-Boot SPL ---------------------------------------------------------------------- KeyError: '_binman_sym_magic'
====================================================================== ERROR: testSymbolsSubsection (binman.ftest.TestFunctional) Test binman can assign symbols from a subsection ---------------------------------------------------------------------- KeyError: '_binman_sym_magic'
====================================================================== FAIL: testExtractAllEntries (binman.ftest.TestFunctional) Test extracting all entries ---------------------------------------------------------------------- AssertionError: 969 != 0
====================================================================== FAIL: testExtractCbfsCompressed (binman.ftest.TestFunctional) Test extracting CBFS compressed data ---------------------------------------------------------------------- AssertionError: 969 != 0
====================================================================== FAIL: testListCmd (binman.ftest.TestFunctional) Test listing the files in an image using an Fdtmap ---------------------------------------------------------------------- AssertionError: Lists differ: ['Nam[463 chars]80 105 u-boot-dtb 80 3c9', [184 chars]bf8'] != ['Nam[463 chars]80 0 u-boot-dtb 80 3c9', [184 chars]bf8']
First differing element 7: ' u-boot-dtb 180 105 u-boot-dtb 80 3c9' ' u-boot-dtb 180 0 u-boot-dtb 80 3c9'
Diff is 888 characters long. Set self.maxDiff to None to see it.
====================================================================== FAIL: testSymbolsTplSection (binman.ftest.TestFunctional) Test binman can assign symbols embedded in U-Boot TPL in a section ---------------------------------------------------------------------- AssertionError: b'\xff\xff\xff\xffBSYM\x04\x00\x00\x00 \x00\x00\x00\x00\x00[36 chars]0klm' != b'\xff\xff\xff\xff56780123456789abcdefghijklm'
====================================================================== FAIL: testSymbolsTplSectionX86 (binman.ftest.TestFunctional) Test binman can assign symbols in a section with end-at-4gb ---------------------------------------------------------------------- AssertionError: b'\xff\xff\xff\xffBSYM\x04\xff\xff\xff \xff\xff\xff\x00\x00[36 chars]0klm' != b'\xff\xff\xff\xff56780123456789abcdefghijklm'
====================================================================== FAIL: testBadSymbolSize (binman.elf_test.TestElf) Test that an attempt to use an 8-bit symbol are detected ---------------------------------------------------------------------- AssertionError: ValueError not raised
====================================================================== FAIL: testDebug (binman.elf_test.TestElf) Check that enabling debug in the elf module produced debug output ---------------------------------------------------------------------- AssertionError: False is not true
====================================================================== FAIL: testNoValue (binman.elf_test.TestElf) Test the case where we have no value for the symbol ---------------------------------------------------------------------- AssertionError: b'BSYM\xff\xff\xff\xff\xff\xff\xff\xff\xff[43 chars]aaaa' != b'aaaaaaaaaaaaaaaaaaaaaaaaaaaa'
====================================================================== FAIL: testOutsideFile (binman.elf_test.TestElf) Test a symbol which extends outside the entry area is detected ---------------------------------------------------------------------- AssertionError: ValueError not raised
====================================================================== FAIL: test_cbfs_arch (binman.cbfs_util_test.TestCbfs) Test on non-x86 architecture ---------------------------------------------------------------------- AssertionError: cbfstool produced a different result
Stdout: diff -y <(xxd -g1 /tmp/expect) <(xxd -g1 /tmp/actual) | colordiff
====================================================================== FAIL: test_cbfs_offset (binman.cbfs_util_test.TestCbfs) Test a CBFS with files at particular offsets ---------------------------------------------------------------------- AssertionError: cbfstool produced a different result
Stdout: diff -y <(xxd -g1 /tmp/expect) <(xxd -g1 /tmp/actual) | colordiff
====================================================================== FAIL: test_cbfs_raw (binman.cbfs_util_test.TestCbfs) Test base handling of a Coreboot Filesystem (CBFS) ---------------------------------------------------------------------- AssertionError: cbfstool produced a different result
Stdout: diff -y <(xxd -g1 /tmp/expect) <(xxd -g1 /tmp/actual) | colordiff
====================================================================== FAIL: test_cbfs_raw_compress (binman.cbfs_util_test.TestCbfs) Test base handling of compressing raw files ---------------------------------------------------------------------- AssertionError: b'' != b'compress xxxxxxxxxxxxxxxxxxxxxx data'
====================================================================== FAIL: test_cbfs_raw_space (binman.cbfs_util_test.TestCbfs) Test files with unused space in the CBFS ---------------------------------------------------------------------- AssertionError: cbfstool produced a different result
Stdout: diff -y <(xxd -g1 /tmp/expect) <(xxd -g1 /tmp/actual) | colordiff
====================================================================== FAIL: test_cbfs_stage (binman.cbfs_util_test.TestCbfs) Tests handling of a Coreboot Filesystem (CBFS) ---------------------------------------------------------------------- AssertionError: cbfstool produced a different result
Stdout: diff -y <(xxd -g1 /tmp/expect) <(xxd -g1 /tmp/actual) | colordiff
====================================================================== SKIP: testCompUtilCompressions (binman.ftest.TestFunctional) Test compression algorithms ---------------------------------------------------------------------- lzma_alone not available
====================================================================== SKIP: testCompUtilPadding (binman.ftest.TestFunctional) Test padding of compression algorithms ---------------------------------------------------------------------- lzma_alone not available
====================================================================== SKIP: testCompUtilVersions (binman.ftest.TestFunctional) Test tool version of compression algorithms ---------------------------------------------------------------------- lzma_alone not available
====================================================================== SKIP: testExtractCbfsRaw (binman.ftest.TestFunctional) Test extracting CBFS compressed data without decompressing it ---------------------------------------------------------------------- lzma_alone not available
---------------------------------------------------------------------- Ran 454 tests in 11.738s
FAILED (failures=15, errors=4, skipped=4)
99% Name Stmts Miss Cover --------------------------------------------------------------------------- tools/binman/__init__.py 0 0 100% tools/binman/bintool.py 254 0 100% tools/binman/btool/btool_gzip.py 5 0 100% tools/binman/btool/bzip2.py 15 0 100% tools/binman/btool/cbfstool.py 24 0 100% tools/binman/btool/fiptool.py 22 0 100% tools/binman/btool/futility.py 24 0 100% tools/binman/btool/ifwitool.py 22 0 100% tools/binman/btool/lz4.py 28 0 100% tools/binman/btool/lzma_alone.py 34 3 91% tools/binman/btool/lzop.py 5 0 100% tools/binman/btool/mkimage.py 29 0 100% tools/binman/btool/xz.py 5 0 100% tools/binman/btool/zstd.py 5 0 100% tools/binman/cbfs_util.py 366 0 100% tools/binman/cmdline.py 73 0 100% tools/binman/control.py 342 0 100% tools/binman/elf.py 195 18 91% tools/binman/entry.py 483 0 100% tools/binman/etype/atf_bl31.py 5 0 100% tools/binman/etype/atf_fip.py 67 0 100% tools/binman/etype/blob.py 39 0 100% tools/binman/etype/blob_dtb.py 46 0 100% tools/binman/etype/blob_ext.py 11 0 100% tools/binman/etype/blob_ext_list.py 32 0 100% tools/binman/etype/blob_named_by_arg.py 9 0 100% tools/binman/etype/blob_phase.py 16 0 100% tools/binman/etype/cbfs.py 101 0 100% tools/binman/etype/collection.py 30 0 100% tools/binman/etype/cros_ec_rw.py 5 0 100% tools/binman/etype/fdtmap.py 62 0 100% tools/binman/etype/files.py 35 0 100% tools/binman/etype/fill.py 13 0 100% tools/binman/etype/fit.py 214 0 100% tools/binman/etype/fmap.py 34 0 100% tools/binman/etype/gbb.py 37 0 100% tools/binman/etype/image_header.py 53 0 100% tools/binman/etype/intel_cmc.py 4 0 100% tools/binman/etype/intel_descriptor.py 39 0 100% tools/binman/etype/intel_fit.py 12 0 100% tools/binman/etype/intel_fit_ptr.py 17 0 100% tools/binman/etype/intel_fsp.py 4 0 100% tools/binman/etype/intel_fsp_m.py 4 0 100% tools/binman/etype/intel_fsp_s.py 4 0 100% tools/binman/etype/intel_fsp_t.py 4 0 100% tools/binman/etype/intel_ifwi.py 67 0 100% tools/binman/etype/intel_me.py 4 0 100% tools/binman/etype/intel_mrc.py 6 0 100% tools/binman/etype/intel_refcode.py 6 0 100% tools/binman/etype/intel_vbt.py 4 0 100% tools/binman/etype/intel_vga.py 4 0 100% tools/binman/etype/mkimage.py 68 0 100% tools/binman/etype/opensbi.py 5 0 100% tools/binman/etype/powerpc_mpc85xx_bootpg_resetvec.py 6 0 100% tools/binman/etype/pre_load.py 77 0 100% tools/binman/etype/scp.py 5 0 100% tools/binman/etype/section.py 376 12 97% tools/binman/etype/tee_os.py 5 0 100% tools/binman/etype/text.py 21 0 100% tools/binman/etype/u_boot.py 7 0 100% tools/binman/etype/u_boot_dtb.py 9 0 100% tools/binman/etype/u_boot_dtb_with_ucode.py 51 0 100% tools/binman/etype/u_boot_elf.py 19 0 100% tools/binman/etype/u_boot_env.py 27 0 100% tools/binman/etype/u_boot_expanded.py 4 0 100% tools/binman/etype/u_boot_img.py 7 0 100% tools/binman/etype/u_boot_nodtb.py 7 0 100% tools/binman/etype/u_boot_spl.py 11 0 100% tools/binman/etype/u_boot_spl_bss_pad.py 14 0 100% tools/binman/etype/u_boot_spl_dtb.py 9 0 100% tools/binman/etype/u_boot_spl_elf.py 7 0 100% tools/binman/etype/u_boot_spl_expanded.py 12 0 100% tools/binman/etype/u_boot_spl_nodtb.py 11 0 100% tools/binman/etype/u_boot_spl_with_ucode_ptr.py 8 0 100% tools/binman/etype/u_boot_tpl.py 11 0 100% tools/binman/etype/u_boot_tpl_bss_pad.py 14 0 100% tools/binman/etype/u_boot_tpl_dtb.py 9 0 100% tools/binman/etype/u_boot_tpl_dtb_with_ucode.py 8 0 100% tools/binman/etype/u_boot_tpl_elf.py 7 0 100% tools/binman/etype/u_boot_tpl_expanded.py 12 0 100% tools/binman/etype/u_boot_tpl_nodtb.py 11 0 100% tools/binman/etype/u_boot_tpl_with_ucode_ptr.py 12 0 100% tools/binman/etype/u_boot_ucode.py 33 0 100% tools/binman/etype/u_boot_with_ucode_ptr.py 42 0 100% tools/binman/etype/vblock.py 38 0 100% tools/binman/etype/x86_reset16.py 7 0 100% tools/binman/etype/x86_reset16_spl.py 7 0 100% tools/binman/etype/x86_reset16_tpl.py 7 0 100% tools/binman/etype/x86_start16.py 7 0 100% tools/binman/etype/x86_start16_spl.py 7 0 100% tools/binman/etype/x86_start16_tpl.py 7 0 100% tools/binman/fip_util.py 202 0 100% tools/binman/fmap_util.py 48 0 100% tools/binman/image.py 164 4 98% tools/binman/state.py 201 0 100% --------------------------------------------------------------------------- TOTAL 4539 37 99%
To get a report in 'htmlcov/index.html', type: python3 -m coverage html Coverage error: 99%, but should be 100% ValueError: Test coverage failure
I guess I just should send a new mail so the community can have a look at it? I sent two patches for low hanging fruits but I'm afraid I won't have time to look at those issues any time soon :/
I'm running Fedora Server 36 x86_64 fully up-to-date if that helps.
[...]
I get 52% coverage only with that exact same branch, something's definitely wrong here in my setup. And I **definitely** do not have tools/binman/etype/mkimage.py listed in there.... Mmmmmm.
You may need to get some bintools with 'binman tool -f missing'. But
It expects apt package manager, I only have dnf :)
Also, tries to download binaries from a Google Drive, no thank you :)
in any case, you only need to worry about coverage in mkimage.py which is what you changed.
That's fair. Thanks for fixing my patches, lemme know if you want a v6 instead.
[...]
I'll play with binman until I manage to get a coverage percentage equal to yours.
OK, I'd appreciate a docs patch if you can produce one from your efforts, or any feedback on how to make this automatic / easy.
I copied the errors I'm having right now a few paragraphs above but the biggest issue was bzip2 getting stuck and messing up the whole test suite. With it patched, I was able to see the line that was kept untested in my patches. Otherwise I believe your advice of running binman test -T was enough if that had worked, just bad luck :)
Thanks! Quentin