Hi Tom,
On 1/20/26 4:47 PM, Tom Rini wrote:
From: Dmitrii Sharshakov <[email protected]>
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 <[email protected]>
Signed-off-by: Andrew Soknacki <[email protected]>
Reviewed-by: Tom Rini <[email protected]>
[trini: Add the test provided by Andrew on IRC, to fix coverage]
Signed-off-by: Tom Rini <[email protected]>
---
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
