Hi Quentin, On Wed, 31 Aug 2022 at 10:39, Quentin Schulz <quentin.sch...@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.sch...@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.sch...@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.sch...@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
