Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImportError when running tests (with editable installs) #1491

Closed
kristapsk opened this issue May 7, 2023 · 9 comments · Fixed by #1484
Closed

ImportError when running tests (with editable installs) #1491

kristapsk opened this issue May 7, 2023 · 9 comments · Fixed by #1484
Labels

Comments

@kristapsk
Copy link
Member

This was first reported by @roshii in #1479. Initially I couldn't reproduce, but at one moment, after recreating jmvenv also started having this problem. It's probably related to pypa/setuptools#3504.

____________________________________________________________________ ERROR collecting jmbitcoin/test/test_ecc_signing.py _____________________________________________________________________
ImportError while importing test module '/home/user/git/joinmarket-clientserver/jmbitcoin/test/test_ecc_signing.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
jmbitcoin/test/test_ecc_signing.py:7: in <module>
    from jmbase import bintohex
E   ImportError: cannot import name 'bintohex' from 'jmbase' (unknown location)
...

For CI opened #1490, which removes -e from requirements files before setting up virtual environment there.

Should figure out what's happening there, how can this be properly solved.

In worst case scenario, old behaviour of --develop setting of install.sh should be restored. Ability to run tests locally is important, but also in-place editing without reinstall each time is very useful in development.

@kristapsk
Copy link
Member Author

Note that this error happens only when running ./test/run_tests.sh. In the same virtual environment JM scripts itself work normally.

@kristapsk
Copy link
Member Author

User on Telegram had this problem when trying JM on FreeBSD. So, this looks like an issue we will need to address soon.

@roshii
Copy link
Contributor

roshii commented Jul 14, 2023

#1484 adresses it. Just let me know if I can do anything to make review easier.

@PulpCattel
Copy link
Member

PulpCattel commented Jul 24, 2023

I recreated the virtualenv and tests pass for me locally with my usual manual procedure (i.e., without using run_tests.sh but rather mostly following this). I suppose this is what was referred to in #1491 (comment).

EDIT: do I need a specific version of something to reproduce this?

@kristapsk
Copy link
Member Author

EDIT: do I need a specific version of something to reproduce this?

Yes, I think so, seems to be related to version of setuptools.

@PulpCattel
Copy link
Member

seems to be related to version of setuptools.

For the record, I'm using setuptools 68.0.0

@roshii
Copy link
Contributor

roshii commented Jul 25, 2023

AFAIK joinmarket is now structured as a legacy python project, designed to be installed in development mode with python setup.py develop command. Not only setuptools sees it as a legacy project, pip as well and most likely other tools.

We could use the transitional compat mode but it will be removed in future versions of setuptools. As such it makes more sense to transition to modern packaging (PEP517, PEP518, etc) soonest possible.

References: https://setuptools.pypa.io/en/latest/userguide/development_mode.html#legacy-behavior
https://peps.python.org/pep-0517
https://peps.python.org/pep-0518

@PulpCattel
Copy link
Member

I think something like #1484 is good even without considering this issue.

@AdamISZ
Copy link
Member

AdamISZ commented Jul 29, 2023

I've spent some time trying to diagnose this (apart from just being a weird bug, it's a big blockage to doing dev work). I was replicating the issue on three different environments, one my home laptop, the other two VMs (Ubuntu 2004 and Debian 5.10), and after messing around a lot with versions of pytest, setuptools and a few other things, it seems like I pinned it down to the pip version.

So taking master, doing install.sh without Qt, on Py3.8 and 3.9, I reproduce this error after:

./install.sh
pip install -r requirements/testing.txt
git clone https://github.com/Joinmarket-Org/miniircd
cp test/regtest_joinmarket.cfg joinmarket.cfg
pytest  --btcconf=/path/to/bitcoin.conf --btcroot=/path/to/bitcoin-24.1/bin/ --btcpwd=x ...

(originally i was doing run_tests.sh but it's better to isolate exact steps here until we figure it out, hence manual steps, and pytest invocation)

But I noticed that on my old JMCS install on my laptop, it works and so I changed various versions. The change in pip version seemed to fix it on all 3 enviroments as above (and as noted, py3.8 and 3.9):

pip install pip==21.3.1

I would note, however, something strange: one of my VMs had defaulted to pip 20.3, whereas the other (the ubuntu one) has 23.x.x, i.e. this version is between the two versions that both didn't work.

But with this change, the editable installs still allow pytest to work:

./install.sh
pip install pip==21.3.1
pip install -r requirements/testing.txt
git clone https://github.com/Joinmarket-Org/miniircd
cp test/regtest_joinmarket.cfg joinmarket.cfg
pytest  --btcconf=/path/to/bitcoin.conf --btcroot=/path/to/bitcoin-24.1/bin/ --btcpwd=x ...

If other people can replicate that it's fixed by pinning the pip version, we could put it into run_tests.sh I guess.

@AdamISZ AdamISZ closed this as completed in b27c86e Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants