cibuildwheel icon indicating copy to clipboard operation
cibuildwheel copied to clipboard

Improve the error message of a misconfigured CIBW_TEST_COMMAND [was "bracex not installed, so pytest fails"]

Open reynoldsnlp opened this issue 4 years ago • 11 comments

I am using joerick/[email protected] and I have two relevant environment variables:

          CIBW_TEST_REQUIRES: pytest
          CIBW_TEST_COMMAND: python -m pytest

pytest is installed correctly, but then when pytest is run, I get this output:

  +  python -m pytest
  ============================= test session starts ==============================
  platform darwin -- Python 3.6.8, pytest-6.2.2, py-1.10.0, pluggy-0.13.1
  rootdir: /Users/runner
  collected 0 items / 1 error
  
  ==================================== ERRORS ====================================
  ________________________ ERROR collecting test session _________________________
  /Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/importlib/__init__.py:126: in import_module
      return _bootstrap._gcd_import(name[level:], package, level)
  <frozen importlib._bootstrap>:994: in _gcd_import
      ???
  <frozen importlib._bootstrap>:971: in _find_and_load
      ???
  <frozen importlib._bootstrap>:955: in _find_and_load_unlocked
      ???
  <frozen importlib._bootstrap>:665: in _load_unlocked
      ???
  /private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/tmpsl686ds5/lib/python3.6/site-packages/_pytest/assertion/rewrite.py:170: in exec_module
      exec(co, module.__dict__)
  work/_actions/joerick/cibuildwheel/v1.10.0/unit_test/main_tests/conftest.py:9: in <module>
      from cibuildwheel import linux, macos, util, windows
  work/_actions/joerick/cibuildwheel/v1.10.0/cibuildwheel/linux.py:9: in <module>
      from .logger import log
  work/_actions/joerick/cibuildwheel/v1.10.0/cibuildwheel/logger.py:8: in <module>
      from cibuildwheel.util import CIProvider, detect_ci_provider
  work/_actions/joerick/cibuildwheel/v1.10.0/cibuildwheel/util.py:15: in <module>
      import bracex
  E   ModuleNotFoundError: No module named 'bracex'
  =========================== short test summary info ============================
  ERROR  - ModuleNotFoundError: No module named 'bracex'
  !!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
  ========================= 1 error in 344.15s (0:05:44) =========================
Error: Command  python -m pytest failed with code 2. None

I see that your setup.cfg has bracex, but for some reason it is not installed when I try to run pytest.

reynoldsnlp avatar Mar 13 '21 20:03 reynoldsnlp

Hmm. bracex is a requirement of cibuildwheel. But your tests run in a different virtualenv from the one that cibuildwheel was installed in, so I wouldn't expect it to be available there. It seems that the pytest discoverer is importing cibuildwheel when it shouldn't. What is your pytest config?

joerick avatar Mar 13 '21 21:03 joerick

I don't really have much other than to declare that the test files are in test/. I have pyproject.toml and setup.cfg, both shown below. No pytest.ini.

pyproject.toml

[build-system]
requires = [
    "setuptools>=42",
    "wheel"
]
build-backend = "setuptools.build_meta"

[project.optional-dependencies]
tests = [
  'pytest',
]

[tool.pytest.ini_options]
minversion = "6.0"
# addopts = "-ra -q"
testpaths = [
    "test",
]

setup.cfg

[metadata]
name = hfst
version = file: VERSION
author = HFST team
author_email = [email protected]
url = https://hfst.github.io/
description = Python interface for HFST
long_description = file: README.rst
long_description_content_type = text/x-rst
license = GNU GPL3
ext_modules=[libhfst_module],
py_modules=['libhfst'],
packages=['hfst', 'hfst.exceptions', 'hfst.sfst_rules',
          'hfst.xerox_rules'],
package_data=package_data,
include_package_data=True,
data_files=[]

[options]
package_dir =
    = src
packages = find:

[options.packages.find]
where = src

[aliases]
test=pytest  # TODO delete this?

[flake8]
doctests = True
ignore = N802,N806
max-complexity = 10

[build_ext]
inplace=1

reynoldsnlp avatar Mar 13 '21 22:03 reynoldsnlp

This only happens on the macos-latest image. Strangely, on the ubuntu-latest image it just doesn't find any of the tests. I'm going to try explicitly running python -m pytest {project}/test.

reynoldsnlp avatar Mar 14 '21 00:03 reynoldsnlp

Adding the path to the test directory fixed the problem, but that seems like a workaround to a bug. pytest should find that directory based on pyproject.toml, and it's strange that cibuildwheel was called at all when the test directory wasn't found. I guess I'm assuming that the working directory for running tests is {project}, but maybe that's not the case?

reynoldsnlp avatar Mar 14 '21 02:03 reynoldsnlp

A few comments on your setup code:

  • Who's picking up project.optional-dependencies? Didn't know anything read project sections in pyproject.toml. If it's just there for future use, that's fine. But curious. It's also not specified in setup.cfg, so you aren't going to be able to install with the [tests] extra in any current version of setuptools. PEP 621 is accepted and now final, but not implemented anywhere yet. (Except cibuildwheel, we pick up project.requires-python, I think we are almost the first / the first to read PEP 621 metadata. :) )

  • You should not specify both packages = [...] and packages = find:. The later is better. [metadata] doesn't have a "packages" field anyway, so it's ignoring that one.

  • Manually forcing build_ext inplace here bothers me.

  • Yes, I agree, I'd remove that alias. No one should be running python setup.py test in 2021. Or really python setup.py <anything>.

Feel free to checkout https://scikit-hep.org/developer/packaging, it might help. It looks pretty good, though, just the points above.


And, for your main question, I'll quote the docs:

To ensure the wheel is imported by your tests (instead of your source copy), tests are run from a different directory. Use the placeholders {project} and {package} when specifying paths in your project

The reason is that if you accidentally build inplace (which is why I don't like the inplace line in the setup), then running from the project directory could possibly pick up your package and pass tests (or you might not have tests that run the compiled code, also possible). So it runs from a different directory. One that seems to accidentally pick up cibuildwheel's checkout in the action mode.

henryiii avatar Mar 14 '21 03:03 henryiii

Maybe we could make a temp dir and run from there to avoid picking up the cibuildwheel checkout. Possibly even add a test_fail.py that contains a single always failing test with an error message "cibuildwheel runs tests from a different directory to ensure your wheel is imported, please use {project} to specify the project directory when running tests".

henryiii avatar Mar 14 '21 03:03 henryiii

Yep, +1 to all henryiii said.

To summarise, you should be doing CIBW_TEST_COMMAND: python -m pytest {project}/tests, since the working dir for running this step isn't your project dir, but some other dir. This ensures that when your tests say import hfst, your library gets imported from the installed wheel, and not the source files in your project.


Maybe we could make a temp dir and run from there to avoid picking up the cibuildwheel checkout

Yes. Good idea. Running pytest like this seems like a too-easy mistake to make. I really wish there was a better way to do this source/wheel isolation... if anyone has any better ideas I'd love to hear them.

Possibly even add a test_fail.py that contains a single always failing test with an error message "cibuildwheel runs tests from a different directory to ensure your wheel is imported, please use {project} to specify the project directory when running tests".

Yeah, I like this too. We can also add a testcase that does CIBW_TEST_COMMAND: python -m pytest and looks for our nice error message.

joerick avatar Mar 14 '21 10:03 joerick

PS: in your specific case, you use src/ layout and are protected from this mistake. But many packages do not use src/ layout, sadly.

henryiii avatar Mar 14 '21 10:03 henryiii

I really like the idea of test_fail.py in a temp dir. That would have helped me immediately fix my problem.

reynoldsnlp avatar Mar 17 '21 18:03 reynoldsnlp

@henryiii Thanks for the comments on my config files. I'm not experienced, so it's nice to have some feedback. Also, thanks for the link to scikit-hep's packaging page. Extremely helpful info there. I wish some of the official docs were so direct.

Regarding the project.optional-dependencies, I just saw it in the PEP, so I used it XD. I guess it's just there for future use ;).

reynoldsnlp avatar Mar 17 '21 18:03 reynoldsnlp

I've been upstreaming parts of that to the official docs. :)

henryiii avatar Mar 17 '21 18:03 henryiii