[PATCH v2 01/11] binman: elf: Check for ELF_TOOLS availability and remove extra semicolon

[Simon Glass]

Hi Lukas,

On Mon, 10 Jul 2023 at 07:49, Lukas Funke
<lukas.funke-oss at weidmueller.com> wrote:
>
> Hi Simon,
>
> On 07.07.2023 19:35, Simon Glass wrote:
> > Hi Lukas,
> >
> > On Thu, 6 Jul 2023 at 09:38, <lukas.funke-oss at weidmueller.com> wrote:
> >>
> >> From: Lukas Funke <lukas.funke at weidmueller.com>
> >>
> >> Check if elf tools are available when running DecodeElf(). Also
> >> remove superfuous semicolon at line ending.
> >>
> >> Signed-off-by: Lukas Funke <lukas.funke at weidmueller.com>
> >> Reviewed-by: Simon Glass <sjg at chromium.org>
> >> ---
> >>
> >> (no changes since v1)
> >>
> >>   tools/binman/elf.py | 10 ++++++----
> >>   1 file changed, 6 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tools/binman/elf.py b/tools/binman/elf.py
> >> index 5816284c32..a53f4b9c4f 100644
> >> --- a/tools/binman/elf.py
> >> +++ b/tools/binman/elf.py
> >> @@ -438,13 +438,15 @@ def DecodeElf(data, location):
> >>       Returns:
> >>           ElfInfo object containing information about the decoded ELF file
> >>       """
> >> +    if not ELF_TOOLS:
> >> +        raise ValueError("Python: No module named 'elftools'")
> >
> > Actually this is missing test coverage. See testEmbedFail() for an
> > example of how to add it for this function.
> >
> > Use 'binman test -T' to see test coverage.
>
> I've extended the test in order to cover the other functions. But
> 'LookupAndWriteSymbols' failed due to different error message:
> "Cannot write symbols to an ELF file without Python elftools" instead of
> "Python: No module named 'elftools'"
>
> Would your prefer to adapt the error message in 'LookupAndWriteSymbols'
> or add another 'assertIn' to cover the additional message? IMHO the
> error messages should be the same.

Yes that's fine, either way. So long as there is test coverage we are good.

Regards,
Simon