Hi Quentin,
On Wed, 31 Aug 2022 at 10:39, Quentin Schulz quentin.schulz@theobroma-systems.com wrote:
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'
This could be a naming difference with the bintools on Fedora.
====================================================================== 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
That is a bit of a mystery.
====================================================================== 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'
Probably the symbol table is not being read correctly in elf.py
====================================================================== 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
There is no particular spec for what cbfstool does, so perhaps you have a different version. Above it shows you have none at all, actually.
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
These are pretty obvious
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 :/
Yes please resend your series, just with the extra test code I added.
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 :)
Patches welcome!
Also, tries to download binaries from a Google Drive, no thank you :)
I don't like it either. Perhaps we could use a web site? I would prefer to build from source, anyway.
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.
Yes please, my things were just an example to help.
[...]
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 :)
OK good.
Regards, Simon