Skip to content

Commit

Permalink
refactor: no placebos, use true Optional
Browse files Browse the repository at this point in the history
For objects that truly might not exist, use Optional.  Some objects will
always exist eventually, and for those we have some null implementation
standins to use without making new placebo classes.
  • Loading branch information
nedbat committed Feb 14, 2023
1 parent d06ace2 commit 026d924
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 50 deletions.
87 changes: 39 additions & 48 deletions coverage/control.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,10 @@ def __init__( # pylint: disable=too-many-arguments
# Other instance attributes, set with placebos or placeholders.
# More useful objects will be created later.
self._debug: DebugControl = NoDebugging()
self._inorout: InOrOut = _InOrOutPlacebo()
self._inorout: Optional[InOrOut] = None
self._plugins: Plugins = Plugins()
self._data: CoverageData = _CoverageDataPlacebo()
self._collector: Collector = _CollectorPlacebo()
self._data: Optional[CoverageData] = None
self._collector: Optional[Collector] = None

self._file_mapper: Callable[[str], str] = abs_file
self._data_suffix = self._run_suffix = None
Expand Down Expand Up @@ -378,6 +378,7 @@ def _should_trace(self, filename: str, frame: FrameType) -> TFileDisposition:
Calls `_should_trace_internal`, and returns the FileDisposition.
"""
assert self._inorout is not None
disp = self._inorout.should_trace(filename, frame)
if self._debug.should('trace'):
self._debug.write(disposition_debug_msg(disp))
Expand All @@ -389,6 +390,7 @@ def _check_include_omit_etc(self, filename: str, frame: FrameType) -> bool:
Returns a boolean: True if the file should be traced, False if not.
"""
assert self._inorout is not None
reason = self._inorout.check_include_omit_etc(filename, frame)
if self._debug.should('trace'):
if not reason:
Expand Down Expand Up @@ -484,12 +486,14 @@ def set_option(self, option_name: str, value: Union[TConfigValueIn, TConfigSecti
def load(self) -> None:
"""Load previously-collected coverage data from the data file."""
self._init()
self._collector.reset()
if self._collector is not None:
self._collector.reset()
should_skip = self.config.parallel and not os.path.exists(self.config.data_file)
if not should_skip:
self._init_data(suffix=None)
self._post_init()
if not should_skip:
assert self._data is not None
self._data.read()

def _init_for_start(self) -> None:
Expand Down Expand Up @@ -541,6 +545,7 @@ def _init_for_start(self) -> None:

self._init_data(suffix)

assert self._data is not None
self._collector.use_data(self._data, self.config.context)

# Early warning if we aren't going to be able to support plugins.
Expand Down Expand Up @@ -584,7 +589,7 @@ def _init_for_start(self) -> None:

def _init_data(self, suffix: Optional[Union[str, bool]]) -> None:
"""Create a data file if we don't have one yet."""
if not self._data._real:
if self._data is None:
# Create the data file. We do this at construction time so that the
# data file will be written into the directory where the process
# started rather than wherever the process eventually chdir'd to.
Expand Down Expand Up @@ -614,6 +619,9 @@ def start(self) -> None:
self._init_for_start()
self._post_init()

assert self._collector is not None
assert self._inorout is not None

# Issue warnings for possible problems.
self._inorout.warn_conflicting_settings()

Expand All @@ -635,6 +643,7 @@ def stop(self) -> None:
if self._instances[-1] is self:
self._instances.pop()
if self._started:
assert self._collector is not None
self._collector.stop()
self._started = False

Expand Down Expand Up @@ -664,10 +673,12 @@ def erase(self) -> None:
"""
self._init()
self._post_init()
self._collector.reset()
if self._collector is not None:
self._collector.reset()
self._init_data(suffix=None)
assert self._data is not None
self._data.erase(parallel=self.config.parallel)
self._data = _CoverageDataPlacebo()
self._data = None
self._inited_for_start = False

def switch_context(self, new_context: str) -> None:
Expand All @@ -686,6 +697,7 @@ def switch_context(self, new_context: str) -> None:
if not self._started: # pragma: part started
raise CoverageException("Cannot switch context, coverage is not started")

assert self._collector is not None
if self._collector.should_start_context:
self._warn("Conflicting dynamic contexts", slug="dynamic-conflict", once=True)

Expand Down Expand Up @@ -791,6 +803,7 @@ def combine(
self._post_init()
self.get_data()

assert self._data is not None
combine_parallel_data(
self._data,
aliases=self._make_aliases(),
Expand All @@ -814,13 +827,15 @@ def get_data(self) -> CoverageData:
self._init_data(suffix=None)
self._post_init()

for plugin in self._plugins:
if not plugin._coverage_enabled:
self._collector.plugin_was_disabled(plugin)
if self._collector is not None:
for plugin in self._plugins:
if not plugin._coverage_enabled:
self._collector.plugin_was_disabled(plugin)

if self._collector.flush_data():
self._post_save_work()
if self._collector.flush_data():
self._post_save_work()

assert self._data is not None
return self._data

def _post_save_work(self) -> None:
Expand All @@ -830,6 +845,9 @@ def _post_save_work(self) -> None:
Look for un-executed files.
"""
assert self._data is not None
assert self._inorout is not None

# If there are still entries in the source_pkgs_unmatched list,
# then we never encountered those packages.
if self._warn_unimported_source:
Expand Down Expand Up @@ -903,6 +921,7 @@ def _analyze(self, it: Union[FileReporter, TMorf]) -> Analysis:

def _get_file_reporter(self, morf: TMorf) -> FileReporter:
"""Get a FileReporter for a module or file name."""
assert self._data is not None
plugin = None
file_reporter: Union[str, FileReporter] = "python"

Expand Down Expand Up @@ -938,6 +957,7 @@ def _get_file_reporters(self, morfs: Optional[Iterable[TMorf]] = None) -> List[F
measured is used to find the FileReporters.
"""
assert self._data is not None
if not morfs:
morfs = self._data.measured_files()

Expand All @@ -952,7 +972,8 @@ def _prepare_data_for_reporting(self) -> None:
"""Re-map data before reporting, to get implicit 'combine' behavior."""
if self.config.paths:
mapped_data = CoverageData(warn=self._warn, debug=self._debug, no_disk=True)
mapped_data.update(self._data, aliases=self._make_aliases())
if self._data is not None:
mapped_data.update(self._data, aliases=self._make_aliases())
self._data = mapped_data

def report(
Expand Down Expand Up @@ -1256,7 +1277,7 @@ def plugin_info(plugins: List[Any]) -> List[str]:
info = [
('coverage_version', covmod.__version__),
('coverage_module', covmod.__file__),
('tracer', self._collector.tracer_name()),
('tracer', self._collector.tracer_name() if self._collector is not None else "-none-"),
('CTracer', 'available' if HAS_CTRACER else "unavailable"),
('plugins.file_tracers', plugin_info(self._plugins.file_tracers)),
('plugins.configurers', plugin_info(self._plugins.configurers)),
Expand All @@ -1267,7 +1288,7 @@ def plugin_info(plugins: List[Any]) -> List[str]:
('config_contents',
repr(self.config._config_contents) if self.config._config_contents else '-none-'
),
('data_file', self._data.data_filename()),
('data_file', self._data.data_filename() if self._data is not None else "-none-"),
('python', sys.version.replace('\n', '')),
('platform', platform.platform()),
('implementation', platform.python_implementation()),
Expand All @@ -1288,44 +1309,14 @@ def plugin_info(plugins: List[Any]) -> List[str]:
('command_line', " ".join(getattr(sys, 'argv', ['-none-']))),
]

info.extend(self._inorout.sys_info())
if self._inorout is not None:
info.extend(self._inorout.sys_info())

info.extend(CoverageData.sys_info())

return info


class _Placebo:
"""Base class for placebos, to prevent calling the real base class __init__."""
def __init__(self) -> None:
...


class _CoverageDataPlacebo(_Placebo, CoverageData):
"""Just enough of a CoverageData to be used when we don't have a real one."""
_real = False

def data_filename(self) -> str:
return "-none-"


class _CollectorPlacebo(_Placebo, Collector):
"""Just enough of a Collector to be used when we don't have a real one."""
def reset(self) -> None:
...

def flush_data(self) -> bool:
return False

def tracer_name(self) -> str:
return "-none-"


class _InOrOutPlacebo(_Placebo, InOrOut):
"""Just enough of an InOrOut to be used when we don't have a real one."""
def sys_info(self) -> Iterable[Tuple[str, Any]]:
return []


# Mega debugging...
# $set_env.py: COVERAGE_DEBUG_CALLS - Lots and lots of output about calls to Coverage.
if int(os.environ.get("COVERAGE_DEBUG_CALLS", 0)): # pragma: debugging
Expand Down
2 changes: 0 additions & 2 deletions coverage/sqldata.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,6 @@ class CoverageData(AutoReprMixin):
"""

_real = True # to distinguish from a placebo in control.py

def __init__(
self,
basename: Optional[FilePath] = None,
Expand Down
1 change: 1 addition & 0 deletions tests/test_oddball.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ def recur(n):
with swallow_warnings("Trace function changed, data is likely wrong: None"):
self.start_import_stop(cov, "recur")

assert cov._collector is not None
pytrace = (cov._collector.tracer_name() == "PyTracer")
expected_missing = [3]
if pytrace: # pragma: no metacov
Expand Down

0 comments on commit 026d924

Please sign in to comment.