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 appending to .coverage if exec ends in non-existent directory #806

Merged
merged 1 commit into from Jul 6, 2019

Conversation

@hemberger
Copy link
Contributor

hemberger commented May 30, 2019

The --append option causes coverage run to fail if the <pyfile> that is executed does a chdir to a directory that is subsequently removed before the exec completes.

For example, with the following file, test.py:

import tempfile
import os

with tempfile.TemporaryDirectory() as tmpdir:
    os.chdir(tmpdir)

Run coverage on this as follows:

python -m coverage run --append test.py

This results in a FileNotFoundError because the SQLite database connector internally tries to find the relative path to the database file, which fails since the cwd does not exist. The traceback looks like:

Traceback (most recent call last):
  File "/usr/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/x/repos/coveragepy/coverage/__main__.py", line 8, in <module>
    sys.exit(main())
  File "/home/x/repos/coveragepy/coverage/cmdline.py", line 762, in main
    status = CoverageScript().command_line(argv)
  File "/home/x/repos/coveragepy/coverage/cmdline.py", line 506, in command_line
    return self.do_run(options, args)
  File "/home/x/repos/coveragepy/coverage/cmdline.py", line 649, in do_run
    self.coverage.save()
  File "/home/x/repos/coveragepy/coverage/control.py", line 553, in save
    data = self.get_data()
  File "/home/x/repos/coveragepy/coverage/control.py", line 607, in get_data
    if self._collector and self._collector.flush_data():
  File "/home/x/repos/coveragepy/coverage/collector.py", line 425, in flush_data
    self.covdata.add_lines(abs_file_dict(self.data))
  File "/home/x/repos/coveragepy/coverage/sqldata.py", line 245, in add_lines
    self._choose_lines_or_arcs(lines=True)
  File "/home/x/repos/coveragepy/coverage/sqldata.py", line 288, in _choose_lines_or_arcs
    with self._connect() as con:
  File "/home/x/repos/coveragepy/coverage/sqldata.py", line 616, in __enter__
    self.connect()
  File "/home/x/repos/coveragepy/coverage/sqldata.py", line 601, in connect
    filename = os.path.relpath(self.filename)
  File "/usr/lib/python3.7/posixpath.py", line 475, in relpath
    start_list = [x for x in abspath(start).split(sep) if x]
  File "/usr/lib/python3.7/posixpath.py", line 383, in abspath
    cwd = os.getcwd()
FileNotFoundError: [Errno 2] No such file or directory

This patch avoids this problem by ensuring that we return to a known directory (the original directory we ran in) once the exec completes.

One could argue that it is the exec'd script's responsibility for ensuring that it doesn't end in an invalid directory, but it seems reasonable for coverage to handle this case since it can do so easily (and because temporary directories are likely to be a common occurrence in a test suite).

I believe this issue also warrants a unit test, but I couldn't make out the right way to do this after a perusal of the test suite. Please feel free to add a test or to guide me to a good example for how to implement it. Thank you!

If the file that is exec'd chdirs to a directory that doesn't exist
at the end of the execution, then we will fail to connect to the
SQLite database (due to a failing `os.getcwd` command).

We can easily fix this if we ensure we are in a directory that
exists after executing the foreign code. Returning to the original
directory seems to be a sensible choice.
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented May 30, 2019

Codecov Report

Merging #806 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #806      +/-   ##
=========================================
- Coverage   90.65%   90.6%   -0.05%     
=========================================
  Files          81      81              
  Lines       11178   11180       +2     
  Branches     1150    1150              
=========================================
- Hits        10133   10130       -3     
- Misses        936     938       +2     
- Partials      109     112       +3
Impacted Files Coverage Δ
coverage/execfile.py 82.75% <100%> (+0.2%) ⬆️
tests/test_arcs.py 98.54% <0%> (-1.46%) ⬇️
coverage/control.py 89.27% <0%> (-0.29%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d469d94...f9f0e65. Read the comment docs.

nedbat added a commit that referenced this pull request Jul 6, 2019
… with an exception
@nedbat nedbat merged commit a9fcee8 into nedbat:master Jul 6, 2019
3 of 4 checks passed
3 of 4 checks passed
Travis CI - Pull Request Build Failed
Details
codecov/patch 100% of diff hit (target 90.65%)
Details
codecov/project Absolute coverage decreased by -0.04% but relative coverage increased by +9.34% compared to d469d94
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@nedbat

This comment has been minimized.

Copy link
Owner

nedbat commented Jul 6, 2019

Thanks! I tweaked the implementation, and added tests, in 9ae35a5

@nedbat

This comment has been minimized.

Copy link
Owner

nedbat commented Jul 16, 2019

This has been released in 5.0a6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.