Combine in finish (extension of #142) #143

ionelmc · 2017-01-03T22:01:22Z

Closes #142. Maybe.

…(where now we reinitialize the cov object for forced combine) that are caused by sloppy cwd changes (crazy stuff ppl put in tests).

…sing data_file attr).

ryanhiebert · 2017-01-03T23:48:58Z

Man, this seems super hacky. What if we made another instance during start, and then just used it for combine at the end, rather than making a new instance at the end? I'll give that a try.

ionelmc · 2017-01-03T23:52:14Z

How would that simplify things? Those paths being made absolute is not that bad, in fact there are many places already doing absolutization in pytest-cov ...

ryanhiebert · 2017-01-03T23:57:38Z

Is that just internally, or is that for reporting? My concern is that codecov won't understand it, and also that test runs on different machines will be needlessly different because of the path.I haven't tried it, so it's possible I just don't understand what's be absolutized.

ryanhiebert · 2017-01-03T23:59:16Z

I did try, before, commit 1b12416, and it gave me this error, which is likely because I'm using tox.ini instead of .coveragerc for my config file:

  File "/Users/ryanhiebert/Code/python/coveragepy/coverage/config.py", line 425, in read_coverage_config
    raise CoverageException("Couldn't read '%s' as a config file" % fname)
coverage.misc.CoverageException: Couldn't read '/Users/ryanhiebert/Code/lektor-git/.coveragerc' as a config file

ionelmc · 2017-01-04T00:01:14Z

Interesting, can you paste full traceback?

ryanhiebert · 2017-01-04T00:02:40Z

Traceback (most recent call last):
  File "/Users/ryanhiebert/Envs/lektor-git/bin/pytest", line 11, in <module>
    sys.exit(main())
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/config.py", line 47, in main
    config = _prepareconfig(args, plugins)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/config.py", line 156, in _prepareconfig
    pluginmanager=pluginmanager, args=args)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 745, in __call__
    return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 339, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 334, in <lambda>
    _MultiCall(methods, kwargs, hook.spec_opts).execute()
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 613, in execute
    return _wrapped_call(hook_impl.function(*args), self.execute)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 250, in _wrapped_call
    wrap_controller.send(call_outcome)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/helpconfig.py", line 32, in pytest_cmdline_parse
    config = outcome.get_result()
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 280, in get_result
    _reraise(*ex)  # noqa
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 265, in __init__
    self.result = func()
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 614, in execute
    res = hook_impl.function(*args)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/config.py", line 910, in pytest_cmdline_parse
    self.parse(args)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/config.py", line 1067, in parse
    self._preparse(args, addopts=addopts)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/config.py", line 1038, in _preparse
    args=args, parser=self._parser)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 745, in __call__
    return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 339, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 334, in <lambda>
    _MultiCall(methods, kwargs, hook.spec_opts).execute()
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 613, in execute
    return _wrapped_call(hook_impl.function(*args), self.execute)
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 254, in _wrapped_call
    return call_outcome.get_result()
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 280, in get_result
    _reraise(*ex)  # noqa
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 265, in __init__
    self.result = func()
  File "/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/_pytest/vendored_packages/pluggy.py", line 614, in execute
    res = hook_impl.function(*args)
  File "/Users/ryanhiebert/Code/python/pytest-cov/src/pytest_cov/plugin.py", line 96, in pytest_load_initial_conftests
    plugin = CovPlugin(ns, early_config.pluginmanager)
  File "/Users/ryanhiebert/Code/python/pytest-cov/src/pytest_cov/plugin.py", line 151, in __init__
    self.start(engine.Central)
  File "/Users/ryanhiebert/Code/python/pytest-cov/src/pytest_cov/plugin.py", line 172, in start
    self.cov_controller.start()
  File "/Users/ryanhiebert/Code/python/pytest-cov/src/pytest_cov/engine.py", line 136, in start
    config_file=self.cov_config)
  File "/Users/ryanhiebert/Code/python/coveragepy/coverage/control.py", line 140, in __init__
    concurrency=concurrency,
  File "/Users/ryanhiebert/Code/python/coveragepy/coverage/config.py", line 425, in read_coverage_config
    raise CoverageException("Couldn't read '%s' as a config file" % fname)
coverage.misc.CoverageException: Couldn't read '/Users/ryanhiebert/Code/lektor-git/.coveragerc' as a config file

ionelmc · 2017-01-04T00:14:59Z

Ah, a slight bug. Fixed now.

ryanhiebert · 2017-01-04T00:16:47Z

src/pytest_cov/engine.py

@@ -18,7 +18,7 @@ def __init__(self, cov_source, cov_report, cov_config, cov_append, config=None,
        """Get some common config used by multiple derived classes."""
        self.cov_source = cov_source
        self.cov_report = cov_report
-        self.cov_config = os.path.abspath(cov_config)
+        self.cov_config = os.path.abspath(cov_config) if os.path.exists(cov_config) else cov_config


But now don't we still have that change in working directory bug if you're not setting the config file manually, and it's not the default .coveragerc (which is used to also mean autodetect)?

If you're not specifying --cov-config then cov_config will be None (and coverage will try to find it itself).

If you do specify it and it doesn't exist then coverage should complain, I suspect it doesn't and we should do our own check.

You're quite correct here, if coverage is using a autofound config file with a different name this will make the coverage reinit malfunction (since config won't exist in cwd).

Actually .... hmmm ... aren't you using --cov-config=tox.ini? That would make the path absolute at the right time ...

No, I'm relying on the autodiscover. I know that pytest-cov is using .coveragerc as the default value from argparse, so this won't ever be None, AFAIK, but it will be .coveragerc, which coverage.py interprets the sameway for backward compatibility.

Creating the combiner instance early along with the other one would also make this issue go away. It does make me wonder if this just really isn't a use-case that combine was intended for.

Actually, I'm sure it was intended for this, at least in part. I just wonder if this is more of a design bug in coverage. It's also something we want to address here, as usage in tox is very common. Still, it does seem cumbersome.

…ggests. This reverts commit 6e097e2 and b55f56d.

ionelmc · 2017-01-04T01:19:00Z

Well if this doesn't work I give up. Today at least.

ryanhiebert · 2017-01-04T02:08:36Z

Nope, not quite. Now it's giving duplicate results, and it's leaving the parallel file, but not leaving the .coverage file:

---------- coverage: platform darwin, python 2.7.12-final-0 ----------
Name                                                                           Stmts   Miss Branch BrPart  Cover
----------------------------------------------------------------------------------------------------------------
src/lektor_git.py                                                                 21      0      6      0   100%
/Users/ryanhiebert/Envs/lektor-git/lib/python2.7/site-packages/lektor_git.py      21      0      6      0   100%
----------------------------------------------------------------------------------------------------------------
TOTAL                                                                             42      0     12      0   100%

ryanhiebert · 2017-01-04T02:47:43Z

src/pytest_cov/engine.py

        self.cov.combine()
        self.cov.save()
+


I think the issue that's causing the duplicates is that you're not setting self.cov = self.combining_cov before doing the load().

ryanhiebert · 2017-01-04T02:49:56Z

src/pytest_cov/engine.py

@@ -221,6 +227,9 @@ def finish(self):

        # Combine all the suffix files into the data file.
        self.cov.stop()
+        self.cov.save()
+        self.cov = self.combining_cov
+        self.cov.load()


I'm not sure this is needed in a distributed mode, unless you're having each worker report itself (which I think is a feature you have). It probably would cause problems in the normal case, though, since I think the parallel version of the coverage file gets deleted normally when it's being combined.

ryanhiebert · 2017-01-04T02:50:58Z

src/pytest_cov/engine.py

@@ -289,3 +298,12 @@ def summary(self, stream):
        """Only the master reports so do nothing."""

        pass
+
+
+def laxabspath(path_or_something, *replacemet):


Is it supposed to be *replacement?

ryanhiebert and others added 6 commits January 1, 2017 23:27

New coverage instance to combine

0625905

Merge remote-tracking branch 'origin/pr/142'

970d198

Load data again before combine.

e77500c

Make sure config path is absulutized early. Fixes failures in finish …

6e097e2

…(where now we reinitialize the cov object for forced combine) that are caused by sloppy cwd changes (crazy stuff ppl put in tests).

Add a test for aliasing (the combine in finish changes).

6e97a39

Seem this becomes absolute path, just let it be.

1b12416

ionelmc mentioned this pull request Jan 3, 2017

Use new coverage instance to combine #142

Closed

Add tests for xdist and suprocess and fix handling in xdist mode (mis…

9881910

…sing data_file attr).

Check if it's a path first (don't absolutize None).

b55f56d

ryanhiebert reviewed Jan 4, 2017

View reviewed changes

ionelmc added 2 commits January 4, 2017 03:17

Create a duplicated (ish) cov object for combining as @ryanhiebert su…

9c34a6f

…ggests. This reverts commit 6e097e2 and b55f56d.

Refactor this into a laxabspath function.

c8b032c

ryanhiebert reviewed Jan 4, 2017

View reviewed changes

ryanhiebert and others added 2 commits January 3, 2017 21:53

Override cov with combining_cov

ea32001

Fix spelling.

8629d1a

ionelmc added the needs more work label May 9, 2017

ionelmc closed this Oct 28, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combine in finish (extension of #142) #143

Combine in finish (extension of #142) #143

ionelmc commented Jan 3, 2017 •

edited

Loading

ryanhiebert commented Jan 3, 2017

ionelmc commented Jan 3, 2017

ryanhiebert commented Jan 3, 2017

ryanhiebert commented Jan 3, 2017

ionelmc commented Jan 4, 2017

ryanhiebert commented Jan 4, 2017

ionelmc commented Jan 4, 2017

ryanhiebert Jan 4, 2017

ionelmc Jan 4, 2017

ionelmc Jan 4, 2017

ryanhiebert Jan 4, 2017

ryanhiebert Jan 4, 2017

ryanhiebert Jan 4, 2017

ionelmc commented Jan 4, 2017

ryanhiebert commented Jan 4, 2017

ryanhiebert Jan 4, 2017

ryanhiebert Jan 4, 2017

ryanhiebert Jan 4, 2017

Combine in finish (extension of #142) #143

Combine in finish (extension of #142) #143

Conversation

ionelmc commented Jan 3, 2017 • edited Loading

ryanhiebert commented Jan 3, 2017

ionelmc commented Jan 3, 2017

ryanhiebert commented Jan 3, 2017

ryanhiebert commented Jan 3, 2017

ionelmc commented Jan 4, 2017

ryanhiebert commented Jan 4, 2017

ionelmc commented Jan 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ionelmc commented Jan 4, 2017

ryanhiebert commented Jan 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ionelmc commented Jan 3, 2017 •

edited

Loading