Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Non-editable test doesn't act as we think #98

Open
garryod opened this issue Dec 9, 2022 · 13 comments
Open

Non-editable test doesn't act as we think #98

garryod opened this issue Dec 9, 2022 · 13 comments
Assignees
Labels
bug Something isn't working

Comments

@garryod
Copy link
Member

garryod commented Dec 9, 2022

As a result of my recent issues transferring a C Extension project to something skeleton-like and @DominicOram's issues around coverage, I ended up looking into how pytest works for non-editable installs; In short form, it doesn't.

To see this yourself, load up the dev container and run the following:

pip uninstall -y python3-pip-skeleton
pip freeze | xargs pip uninstall -y
pip install .[dev]
mv src src.hidden
pytest

You will get the following output:

/venv/lib/python3.11/site-packages/coverage/report.py:87: CoverageWarning: Couldn't parse '/scratch/enu43627/projects/python3-pip-skeleton/src/python3_pip_skeleton/__init__.py': No source for code: '/scratch/enu43627/projects/python3-pip-skeleton/src/python3_pip_skeleton/__init__.py'. (couldnt-parse)
  coverage._warn(msg, slug="couldnt-parse")

This is because pytest uses it's own import machinery which is reading the files from the src directory regardless of the install being "editable", resulting in very fragile behaviour which is not representative of shipped package

@garryod garryod added the bug Something isn't working label Dec 9, 2022
@garryod garryod self-assigned this Dec 9, 2022
@garryod
Copy link
Member Author

garryod commented Dec 9, 2022

After some discussion with @AlexanderWells-diamond the issue seems to be resolvable with edits to the configuration. Specifically by adding a .coveragerc which sets source = ["src", "**/site-packages/"] and setting --cov-config=.converagerc in in the pytest config. I will look into how we can more cleanly implement these config options and submit a PR once I have a satisfactory resolution

@GDYendell
Copy link
Collaborator

GDYendell commented Dec 9, 2022

Does this not work then? https://github.com/DiamondLightSource/python3-pip-skeleton/blob/main/pyproject.toml#L86

[tool.coverage.paths]
# Tests are run from installed location, map back to the src directory
source = ["src", "**/site-packages/"]

@garryod
Copy link
Member Author

garryod commented Dec 9, 2022

It doesn't appear to, there seems to be something wrong with how coverage parses source from a toml. Only the first path in the list is used.

@coretl
Copy link
Contributor

coretl commented Dec 9, 2022

Coverage.py will read from “pyproject.toml” if TOML support is available, either because you are running on Python 3.11 or later, or because you installed with the toml extra (pip install coverage[toml]). Configuration must be within the [tool.coverage] section, for example, [tool.coverage.run]. Environment variable expansion in values is available, but only within quoted strings, even for non-string values.

https://coverage.readthedocs.io/en/6.5.0/config.html

Is the toml extra for coverage relevant? Or will it already have what it needs from another dep?

Might it prefer a multiline string to a list of strings?

@garryod
Copy link
Member Author

garryod commented Dec 9, 2022

Is the toml extra for coverage relevant? Or will it already have what it needs from another dep?

Unfortunately not this, pytest-cov depends on coverage[toml]

Might it prefer a multiline string to a list of strings?

It expects a list, when given a multiline string the following error is raised:

ValueError: Option 'source' in section 'tool.coverage.paths' is not a list: 'src*\n*/site-packages/\n'

@coretl
Copy link
Contributor

coretl commented Dec 9, 2022

@coretl
Copy link
Contributor

coretl commented Dec 9, 2022

After some discussion with @AlexanderWells-diamond the issue seems to be resolvable with edits to the configuration. Specifically by adding a .coveragerc which sets source = ["src", "**/site-packages/"] and setting --cov-config=.converagerc in in the pytest config. I will look into how we can more cleanly implement these config options and submit a PR once I have a satisfactory resolution

Does this actually fix it in this case? Or does it only work with the editable install?

@garryod
Copy link
Member Author

garryod commented Dec 9, 2022

After some discussion with @AlexanderWells-diamond the issue seems to be resolvable with edits to the configuration. Specifically by adding a .coveragerc which sets source = ["src", "**/site-packages/"] and setting --cov-config=.converagerc in in the pytest config. I will look into how we can more cleanly implement these config options and submit a PR once I have a satisfactory resolution

Does this actually fix it in this case? Or does it only work with the editable install?

This fixes it.
Edit: It seems to cause issue when src/ exists and tests are run on the non-editable install, but works when src/ is absent

@coretl
Copy link
Contributor

coretl commented Dec 9, 2022

Looks like the coverage maintainer uses a similar setup (but not using pytest-cov):
nedbat/coveragepy#1415 (comment)

What about if we try

[tool.coverage.paths]
source = ["src", "*/site-packages"]   

@coretl
Copy link
Contributor

coretl commented Dec 9, 2022

It's also worth trying both pytest and python -m pytest as they have subtly different sys.path values...

@garryod
Copy link
Member Author

garryod commented Dec 9, 2022

Okay, so using .coveragerc with source = ["src", "*/site-packages"] for tests I get the follow result:

  • -e .[dev] works when src/ is present, but fails when src/ is missing - expected
  • .[dev] works regardless of src/

@garryod
Copy link
Member Author

garryod commented Dec 9, 2022

When switching over to pyproject.toml with source = ["src", "*/site-packages"], I get the following results:

  • -e .[dev] works when src/ is present, but fails when src/ is missing - expected
  • .[dev] works when src/ is present but fails when src/ is missing

Using source = ["*/site-packages", "src"]

  • -e .[dev] fails regardless of whether src/ is present - though for different reasons
  • .[dev] works regardless of src/

@garryod
Copy link
Member Author

garryod commented Dec 9, 2022

From discussion with @coretl, there seems to be two issues here:

  • coverage reads the config differently from .coveragerc and pyproject.toml, it seems as if only the first path in source is loaded from the toml. This will require fixes upstream
  • The _version.py file generated by setuptools_scm is included in such a way that it causes an ImportPathMismatchError on non-editable installs. This can be fixed with PY_IGNORE_IMPORTMISMATCH=1
    pytest.pathlib.ImportPathMismatchError: ('python3_pip_skeleton._version', '/venv/lib/python3.11/site-packages/python3_pip_skeleton/_version.py', PosixPath('/scratch/enu43627/projects/python3-pip-skeleton/src/python3_pip_skeleton/_version.py'))

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants