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

fix: runpy.run_path(path) crashes with pathlib.Path #1819

Merged
merged 8 commits into from
Jul 19, 2024

Conversation

askhl
Copy link
Contributor

@askhl askhl commented Jul 18, 2024

I ran into the following problem where coverage replaces runpy.run_path() in a way that does not accept Path objects:

import runpy

script = "print('hello, world!')\n"

def test_path(tmp_path):
    pyfile = tmp_path / 'script.py'
    pyfile.write_text(script)
    runpy.run_path(pyfile)

This passes with ordinary pytest but crashes with pytest --cov with the following traceback:

Traceback (most recent call last):
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/_pytest/runner.py", line 341, in from_call
    result: Optional[TResult] = func()
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/flaky/flaky_pytest_plugin.py", line 146, in <lambda>
    lambda: ihook(item=item, **kwds), when=when, reraise=reraise
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/pluggy/_callers.py", line 182, in _multicall
    return outcome.get_result()
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/pluggy/_result.py", line 100, in get_result
    raise exc.with_traceback(exc.__traceback__)
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/_pytest/runner.py", line 177, in pytest_runtest_call
    raise e
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/_pytest/runner.py", line 169, in pytest_runtest_call
    item.runtest()
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/_pytest/python.py", line 1792, in runtest
    self.ihook.pytest_pyfunc_call(pyfuncitem=self)
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/pluggy/_callers.py", line 139, in _multicall
    raise exception.with_traceback(exception.__traceback__)
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
  File "/home/askhl/install/pyenv/lib/python3.10/site-packages/_pytest/python.py", line 194, in pytest_pyfunc_call
    result = testfunction(**testargs)
  File "/home/askhl/coverage-runpy/test.py", line 11, in test_path
    runpy.run_path(pyfile)
  File "/usr/lib/python3.10/runpy.py", line 289, in run_path
    return _run_module_code(code, init_globals, run_name,
  File "/usr/lib/python3.10/runpy.py", line 96, in _run_module_code
    _run_code(code, mod_globals, init_globals,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/askhl/src/coveragepy/coverage/control.py", line 395, in _should_trace
    disp = self._inorout.should_trace(filename, frame)
  File "/home/askhl/src/coveragepy/coverage/inorout.py", line 324, in should_trace
    filename = source_for_file(dunder_file)
  File "/home/askhl/src/coveragepy/coverage/python.py", line 107, in source_for_file
    if filename.endswith(".py"):
AttributeError: 'PosixPath' object has no attribute 'endswith'

This PR contains a minimal fix which sends the input through str(Path(...)). However a proper fix might require some awareness of the surrounding code as well as a test, so I'd expect the PR in its current form to be insufficient. Feedback would be appreciated.

@nedbat
Copy link
Owner

nedbat commented Jul 18, 2024

Thanks, I see there is a problem. I'd want to understand the full call stack to know where I should be accepting Path|str instead of just str.

@askhl
Copy link
Contributor Author

askhl commented Jul 18, 2024

Thanks @nedbat!

It crossed my mind that it could originate with pytest or pytest-cov, since there's something finicky going on between runpy and the code object that could originate from some out-of-sight hook. I reproduced it now with only coverage run:

import runpy
from pathlib import Path

script = "print('hello, world!')\n"
pyfile = Path('script.py')
pyfile.write_text(script)
runpy.run_path(pyfile)

This yields the considerably shorter stacktrace

(pyenv) askhl@alberich:~/coverage-runpy$ coverage run test.py 
Traceback (most recent call last):
  File "/home/askhl/coverage-runpy/test.py", line 8, in <module>
    runpy.run_path(pyfile)
  File "/usr/lib/python3.10/runpy.py", line 289, in run_path
    return _run_module_code(code, init_globals, run_name,
  File "/usr/lib/python3.10/runpy.py", line 96, in _run_module_code
    _run_code(code, mod_globals, init_globals,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/askhl/src/coveragepy/coverage/control.py", line 395, in _should_trace
    disp = self._inorout.should_trace(filename, frame)
  File "/home/askhl/src/coveragepy/coverage/inorout.py", line 324, in should_trace
    filename = source_for_file(dunder_file)
  File "/home/askhl/src/coveragepy/coverage/python.py", line 107, in source_for_file
    if filename.endswith(".py"):
AttributeError: 'PosixPath' object has no attribute 'endswith'

After some messing around I found that usually the function receives a str, but it receives a Path('script.py') when it does dunder_file = frame.f_globals and frame.f_globals.get("__file__").

I pushed a change inserting a conversion there instead of in the previous place.

So that fix will equally well solve the problem without the need to think about Path|str in multiple places, and hence is a more credible fix.

But I don't know if there's any documentation that could reveal what types frame.f_globals.get('__file__') should have.

@nedbat
Copy link
Owner

nedbat commented Jul 18, 2024

It crossed my mind that it could originate with pytest or pytest-cov

No worries, when a bug report mentions pytest-cov, the first thing I do is try to reproduce it without it. I also saw it with just coverage run.

I like the latest change: it feels like the right place.

Do you want to add yourself to CONTRIBUTORS and add a changelog entry? I can take care of it if you like.

@askhl askhl marked this pull request as ready for review July 18, 2024 23:02
@askhl
Copy link
Contributor Author

askhl commented Jul 18, 2024

I have added a test in what I perceive to be a reasonably correct place.

Do you want to add yourself to CONTRIBUTORS and add a changelog entry? I can take care of it if you like.

Thanks. Since the list of unreleased changes is empty, I was not sure whether to add a note before or after .. scriv-start-here, or whether that's necessarily the correct place. Please make any changes you think appropriate.

@askhl
Copy link
Contributor Author

askhl commented Jul 19, 2024

I just pushed a lint-fixing change but you have apparently already done something about it. Feel free to ignore/adapt/close this.

I am not sure whether this should actually be considered an issue with runpy. The docs (https://docs.python.org/3/reference/import.html#file__) say rather clearly that __file__ should be str if it exists.

Thanks a lot!

@nedbat
Copy link
Owner

nedbat commented Jul 19, 2024

I also beefed up the test a bit in askhl#1. You can merge that pull request to get my changes.

@askhl
Copy link
Contributor Author

askhl commented Jul 19, 2024

I also beefed up the test a bit in askhl#1. You can merge that pull request to get my changes.

Nice. It's merged.

@nedbat nedbat merged commit 7fb846e into nedbat:master Jul 19, 2024
35 checks passed
@nedbat
Copy link
Owner

nedbat commented Jul 19, 2024

Thanks!

@askhl askhl deleted the fix-runpy-pathlib branch July 19, 2024 17:31
@nedbat
Copy link
Owner

nedbat commented Aug 4, 2024

This is now released as part of coverage 7.6.1.

github-merge-queue bot pushed a commit to rustymotors/server that referenced this pull request Aug 4, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [coverage](https://github.com/nedbat/coveragepy) | `==7.6.0` ->
`==7.6.1` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/coverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/coverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/coverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/coverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>nedbat/coveragepy (coverage)</summary>

###
[`v7.6.1`](https://github.com/nedbat/coveragepy/blob/HEAD/CHANGES.rst#Version-761--2024-08-04)

[Compare
Source](https://github.com/nedbat/coveragepy/compare/7.6.0...7.6.1)

- Fix: coverage used to fail when measuring code using
:func:`runpy.run_path <python:runpy.run_path>` with a :class:`Path
<python:pathlib.Path>` argument.
    This is now fixed, thanks to `Ask Hjorth Larsen <pull 1819_>`\_.

- Fix: backslashes preceding a multi-line backslashed string could
confuse the
HTML report. This is now fixed, thanks to `LiuYinCarl <pull 1828_>`\_.

- Now we publish wheels for Python 3.13, both regular and free-threaded.

.. \_pull
1819:[nedbat/coveragepy#1819
.. \_pull
1828[nedbat/coveragepy#1828

.. \_changes\_7-6-0:

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/rustymotors/server-old).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
renovate bot added a commit to allenporter/flux-local that referenced this pull request Aug 5, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [coverage](https://github.com/nedbat/coveragepy) | `==7.6.0` ->
`==7.6.1` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/coverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/coverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/coverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/coverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>nedbat/coveragepy (coverage)</summary>

###
[`v7.6.1`](https://github.com/nedbat/coveragepy/blob/HEAD/CHANGES.rst#Version-761--2024-08-04)

[Compare
Source](https://github.com/nedbat/coveragepy/compare/7.6.0...7.6.1)

- Fix: coverage used to fail when measuring code using
:func:`runpy.run_path <python:runpy.run_path>` with a :class:`Path
<python:pathlib.Path>` argument.
    This is now fixed, thanks to `Ask Hjorth Larsen <pull 1819_>`\_.

- Fix: backslashes preceding a multi-line backslashed string could
confuse the
HTML report. This is now fixed, thanks to `LiuYinCarl <pull 1828_>`\_.

- Now we publish wheels for Python 3.13, both regular and free-threaded.

.. \_pull
1819:[nedbat/coveragepy#1819
.. \_pull
1828[nedbat/coveragepy#1828

.. \_changes\_7-6-0:

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/allenporter/flux-local).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
fritterhoff pushed a commit to eduMFA/eduMFA that referenced this pull request Aug 5, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [test/coverage](https://github.com/nedbat/coveragepy) | `==7.6.0` ->
`==7.6.1` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/test%2fcoverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/test%2fcoverage/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/test%2fcoverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/test%2fcoverage/7.6.0/7.6.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>nedbat/coveragepy (test/coverage)</summary>

###
[`v7.6.1`](https://github.com/nedbat/coveragepy/blob/HEAD/CHANGES.rst#Version-761--2024-08-04)

[Compare
Source](https://github.com/nedbat/coveragepy/compare/7.6.0...7.6.1)

- Fix: coverage used to fail when measuring code using
:func:`runpy.run_path <python:runpy.run_path>` with a :class:`Path
<python:pathlib.Path>` argument.
    This is now fixed, thanks to `Ask Hjorth Larsen <pull 1819_>`\_.

- Fix: backslashes preceding a multi-line backslashed string could
confuse the
HTML report. This is now fixed, thanks to `LiuYinCarl <pull 1828_>`\_.

- Now we publish wheels for Python 3.13, both regular and free-threaded.

.. \_pull
1819:[nedbat/coveragepy#1819
.. \_pull
1828[nedbat/coveragepy#1828

.. \_changes\_7-6-0:

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job log](https://developer.mend.io/github/eduMFA/eduMFA).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants