Confusing error message when pyvex fails to import

Question

Confusing error message when pyvex fails to import

cdisselkoen opened this issue 6 years ago · 5 comments

If pyvex fails to import, archinfo/arch.py sets _pyvex to None (line 13). The rest of the code is very careful to check that _pyvex is not None before using it, except line 188, which references _pyvex without first checking that it is not None. As a result, a failed import of pyvex gives the error message 'AttributeError: 'NoneType' object has no attribute 'vex_ffi' on line 188, which is a very confusing way to say "pyvex failed to import". I'm not sure whether the proper fix is to simply move the two lines 188-189 inside the if-block which begins at line 191? or if something more complicated is needed. If the former, I'm happy to put up a PR.

Answer 1 · 2019-04-02T01:59:23.000Z

It is indeed a bug. I would move and _pyvex is not None four two lines up. PR welcome!

Also, it would be great if you can add a test case to test the import of archinfo without pyvex.

Answer 2 · 2019-04-02T02:20:37.000Z

How on earth can that be tested???? That would involve some hooking into python’s import mechanism?

…

On Mon, Apr 1, 2019 at 6:59 PM Fish ***@***.***> wrote: It is indeed a bug. I would move and _pyvex is not None four two lines up. PR welcome! Also, it would be great if you can add a test case to test the import of archinfo without pyvex. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#62 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACYg9YjC4f3Efl4P3nc_ctvN7ipog5cXks5vcrl7gaJpZM4cW6Rv> .

Answer 3 · 2019-04-02T04:02:15.000Z

Actually, https://medium.com/python-pandemonium/how-to-test-your-imports-1461c1113be1 seems to have a solution for how to create a test case. I'm on it

Answer 4 · 2019-04-02T04:05:57.000Z

... wait, where are the tests for this repo?

Answer 5 · 2019-04-02T08:55:11.000Z

Great. We never bother to write any test cases for archinfo. You may create a tests directory and drop your test file there. We will add it to our CI later. Thanks in advance!