initial DebugControl support

[cdleonard]

Fixes #7, partially

Only covers the front-end CoverageParserThread PatchedPopen ShellPlugin classes. Now that infrastructure is in place it can be extended to cover more classes.

I only added enough debugging to investigate #41

Debug option names are debatable, I have a personal bias for longer names.

New tests are required to maintain 100% coverage, those use unittest.mock, not yet present in this project. There are a bunch of other manual fakes/mocks that could be replaced using unittest.mock, like CovWriterFake.

The coveragepy package seems to treat the DebugControl class as somewhat private so it's possible that using it this way will cause issues when running against different versions of coveragepy. For example I had to fetch Coverage._debug which is a private member.

[cdleonard]

CI fails because the _debug_write is not actually called under test.

[lackhove]

@cdleonard Very nice, thanks! I am a little hesitant about this due to the private member access. I don't want to have to keep up with upstream changes just to keep coverage-sh working, and I would also like to avoid having to test against multiple coverage-py versions.

I see two ways of taking this forward: asking Ned to make the debug API public (not sure how he feels about this, and it will probably take a while), or isolating the debug code and simply doing nothing if it doesn't work. What do you think?

Regarding the coverage: I am absolutely fine with reducing the limit, e.g. to 80%. You don't have to go to great lengths just to keep the (silly) 100%.

[lackhove]

@cdleonard: I think instead of creating DebugControl instances in PatchedPopen and ShellPlugin and passing them down to the other classes, we should create one module level instance and use that, similar to how stdlibs logging module is used. This would simplify the code and honor that logging is an external resource with its own lifecycle that cannot be owned by a class.

Instead of a DebugControl instance we could create a facade around DebugControl that implements the aforementioned try...except logic and maybe other workarounds (e.g. locking for thread safety) and instantiate that.

[cdleonard]

The reason I passed a DebugControl instance into classes is because this is what coveragepy does internally. I also copied the pattern of passing a DebugControlString instead when testing classes stand-alone.

I considered creating a DebugControl instance at plugin configure time but:

• PatchedPopen doesn't have access to ShellPlugin (I guess ShellPlugin could initialize a global at configure time).
• I'm not sure multiple DebugControl instances will function correctly in all circumstances. What if coverage_sh and coveragepy attempt writing to the same file?

Having a module-level logger is what I usually do, just following the standard logger = logging.getLogger(__name__) pattern. Here I deliberately tried to imitate coveragepy.

We could try to post a patch to coveragepy which exposes Coverage.debug instance or maybe passes it to Plugin.configure and see what is the reaction.

Regading testing versus multiple coveragepy versions - that would be worthwhile by itself.

[lackhove]

@cdleonard: Thats what i had in mind: https://github.com/lackhove/coverage-sh/tree/feature/debug-control-facade

it just uses the current Coverage object's DebugControl instance if thats available and does nothing / warns in case anything goes wrong. The way self is passed down isnt pretty either, but at least this way the debug logging is safe against internal API changes, does not clutter the plugin classes, is generic and testing does not require monkeypatching. What do you think?

[cdleonard]

It seems fine.

• It does preserve the ability to use the COVERAGE_DEBUG envvar which is very nice and useful.
• It's slightly slower. At least the "inspect" stuff should go inside the if debug_control.should block
• There's less boilerplate code which is nice.
• One advantage of passing the DebugControl object (like coveragepy does) is that you can easily pass a custom object from test to capture messages. But I've never had trouble using approaches like pytest.LoggingCaptureFixture.

The coveragepy internal debug logging system seems a bit over-engineered, your approach of a single debug_write method is good enough.

I would also rebase/rearrange but I noticed that you've been squashing PRs on merge anyway.

...