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:

Hi Simon,

On 8/27/22 02:21, Simon Glass wrote:
Hi Quentin,

On Fri, 26 Aug 2022 at 09:37, Quentin Schulz <foss+ub...@0leil.net> wrote:

From: Quentin Schulz <quentin.sch...@theobroma-systems.com>

Some image types handled by mkimage require the datafiles to be passed
independently (-d data1:data2) for specific handling of each. A
concatenation of datafiles prior to passing them to mkimage wouldn't
work.

That is the case for rkspi for example which requires page alignment
and only writing 2KB every 4KB.

This adds the ability to tell binman to pass the datafiles without
prior concatenation to mkimage, by adding the multiple-data-files
boolean property to the mkimage node.

Cc: Quentin Schulz <foss+ub...@0leil.net>
Reviewed-by: Simon Glass <s...@chromium.org>
Signed-off-by: Quentin Schulz <quentin.sch...@theobroma-systems.com>
---

v5:
   - changed to use full path from input dir with tools.get_input_filename
   to make it possible to run the unit tests,
   - added unit test,


   tools/binman/entries.rst                      | 22 ++++++++++
   tools/binman/etype/mkimage.py                 | 41 +++++++++++++++++--
   tools/binman/ftest.py                         | 16 ++++++++

Please put the new test at the end.

   .../test/241_mkimage_multiple_data_files.dts  | 21 ++++++++++
   4 files changed, 96 insertions(+), 4 deletions(-)
   create mode 100644 tools/binman/test/241_mkimage_multiple_data_files.dts

This is pretty close but it still missing a line of test coverage.
Please try 'binman test -T' to see it. I'd also prefer a shorter

This does not work on Fedora.
1) there's no python3-coverage binary available,
2) After replacing python3-coverage with just coverage, the tests are
stuck and never finish, (I have seen the patches to use COVERAGE
environment variable so I guess the required changes might be tackled
soon in master),

Any tip on how to identify which test is stuck except going through them
one by one?

One way is to add comment blocks '''...''' across the ftest.py file,
using a binary chop to identify the problem.

Or, since tests are run in series, you could hack test_util to pass
verbose parameters when it runs the tests - see 'cmd =' in
run_test_coverage().


I just commented out tests and found the following two are failing on my system:
testCompUtilVersions and testListBintools.

After digging a bit it seems that it is stuck here: https://source.denx.de/u-boot/u-boot/-/blob/master/tools/patman/command.py#L105 for bzip2.

Furthermore:
bzip2 -V > /dev/null
bzip2 -V > /dev/null 2>&1
both get stuck which I assume is where the issue lies :)

bzip2 --help is just fine BTW.

I tested on a colleague's PC running Ubuntu 22.04.1, it works as intended. I guess I'll have to check if Fedora or Ubuntu has patches on top of bzip2 source code that triggers/patches this behavior.


python3-coverage is also not available in the container image built from
tools/docker/Dockerfile.

does 'python3 -m coverage' work?


diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index 0f6d1aa902..eaa769a564 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -58,11 +58,11 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
     prefix = ''
     if build_dir:
prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
-    cmd = ('%spython3-coverage run '
+    cmd = ('%spython3 -m coverage run '
            '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
prog, extra_args or '', test_cmd))
     os.system(cmd)
-    stdout = command.output('python3-coverage', 'report')
+    stdout = command.output('python3', '-m', 'coverage', 'report')
     lines = stdout.splitlines()
     if required:
         # Convert '/path/to/name.py' just the module name 'name'
@@ -81,7 +81,7 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
     print(coverage)
     if coverage != '100%':
         print(stdout)
- print("To get a report in 'htmlcov/index.html', type: python3-coverage html") + print("To get a report in 'htmlcov/index.html', type: python3 -m coverage html")
         print('Coverage error: %s, but should be 100%%' % coverage)
         ok = False
     if not ok:

works just fine for me.

Michal Suchánek seems to disagree with me on this one, see https://lore.kernel.org/u-boot/20220830101149.gm28...@kitsune.suse.cz/

or this:

https://urldefense.proofpoint.com/v2/url?u=https-3A__coverage.readthedocs.io_en_6.3.2_install.html&d=DwIBaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=AH6ijvL2fp8TDrFjgeja0AVykFkzBjRPDOAfg8m_eKMHJW7RzTVA1wMpGn7qBwOe&s=BLW968ZKOcdPWg0s4-4AlA_rqiJCCCKPjP-Y-Fux6oI&e=


filename for the 241 file.

I've pushed a tree containing a suggested fix (updating this patch). I
can update it when applying if you like, otherwise please send a new
version.


Where did you push the tree?

Sorry I forgot to mention that:

https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_sjg20_u-2Dboot_tree_try-2Drk4&d=DwIBaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=AH6ijvL2fp8TDrFjgeja0AVykFkzBjRPDOAfg8m_eKMHJW7RzTVA1wMpGn7qBwOe&s=7LOQoSkcQA52SvFgC_aUR4l2MtMWjdVM-t_bCKUetEs&e=



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?

Cheers,
Quentin

Reply via email to