[PATCH v3] binman: elf: Check for ELF_TOOLS availability in is_valid

[Quentin Schulz]

Hi Tom,

On 1/20/26 4:47 PM, Tom Rini wrote:
> From: Dmitrii Sharshakov <d3dx12.xx at gmail.com>
> 
> Check if elftools package is available before running DecodeElf().
> 
> This clarifies the error message and adds the required test for
> coverage.
> 
> Signed-off-by: Dmitrii Sharshakov <d3dx12.xx at gmail.com>
> Signed-off-by: Andrew Soknacki <asoknacki at gmail.com>
> Reviewed-by: Tom Rini <trini at konsulko.com>
> [trini: Add the test provided by Andrew on IRC, to fix coverage]
> Signed-off-by: Tom Rini <trini at konsulko.com>
> ---
> Changes in v3:
> - Add the test, provided by Andrew, to address the coverage failure in
>    CI
> ---
>   tools/binman/elf.py      |  2 ++
>   tools/binman/elf_test.py | 12 ++++++++++++
>   2 files changed, 14 insertions(+)
> 
> diff --git a/tools/binman/elf.py b/tools/binman/elf.py
> index 6ac960e04196..899c84ad36d6 100644
> --- a/tools/binman/elf.py
> +++ b/tools/binman/elf.py
> @@ -570,6 +570,8 @@ def is_valid(data):
>       Returns:
>           bool: True if a valid Elf file, False if not
>       """
> +    if not ELF_TOOLS:
> +        raise ValueError("Python: No module named 'elftools'")
>       try:
>           DecodeElf(data, 0)
>           return True
> diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py
> index 5b1733928986..3ad0bf4c4b09 100644
> --- a/tools/binman/elf_test.py
> +++ b/tools/binman/elf_test.py
> @@ -373,6 +373,18 @@ class TestElf(unittest.TestCase):
>           self.assertEqual(True, elf.is_valid(data))
>           self.assertEqual(False, elf.is_valid(data[4:]))
>   
> +    def test_is_valid_fail(self):
> +        """Test calling is_valid() without elftools"""
> +        old_val = elf.ELF_TOOLS
> +        try:
> +            elf.ELF_TOOLS = False
> +            with self.assertRaises(ValueError) as e:
> +                elf.is_valid(b'')
> +            self.assertIn("Python: No module named 'elftools'",
> +                      str(e.exception))
> +        finally:
> +            elf.ELF_TOOLS = old_val
> +

I'm not sure this is a good idea. The issue is that you're modifying the 
variable from a python module. The check may be run by other tests at 
the same time in different threads and I think the tests will 
unnecessarily be skipped or fail?

I think we may want something like:

@unittest.mock.patch('binman.elf.ELF_TOOLS', False)
def test_is_valid_fail(self):
?

Cheers,
Quentin