Skip to content

Commit 2a383b0

Browse files
fix: coverage code quality improvements
- Use getattr() for betterproto oneof field access (prevents AttributeError) - Fix _is_user_file path prefix collision (/app matching /application) - Add os.path.realpath() for symlink-safe path comparison - Add thread lock to start_coverage_collection() - Add double-init guard (stop existing instance before creating new) - Narrow */test* omit pattern to */tests/* and */test_*.py - Log failed file analysis at debug level instead of silent swallow
1 parent 5d438df commit 2a383b0

2 files changed

Lines changed: 35 additions & 22 deletions

File tree

drift/core/communication/communicator.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -781,8 +781,8 @@ def _background_read_loop(self) -> None:
781781

782782
def _handle_set_time_travel_sync(self, cli_message: CliMessage) -> None:
783783
"""Handle SetTimeTravel request from CLI and send response."""
784-
request = cli_message.set_time_travel_request
785-
if not request:
784+
request = getattr(cli_message, "set_time_travel_request", None)
785+
if request is None:
786786
return
787787

788788
logger.debug(
@@ -818,7 +818,7 @@ def _handle_set_time_travel_sync(self, cli_message: CliMessage) -> None:
818818

819819
def _handle_coverage_snapshot_sync(self, cli_message: CliMessage) -> None:
820820
"""Handle CoverageSnapshot request from CLI and send response."""
821-
request = cli_message.coverage_snapshot_request
821+
request = getattr(cli_message, "coverage_snapshot_request", None)
822822
if request is None:
823823
return
824824

drift/core/coverage_server.py

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@
2727

2828

2929
def start_coverage_collection() -> bool:
30-
"""Initialize coverage.py collection if NODE_V8_COVERAGE is set.
30+
"""Initialize coverage.py collection if TUSK_COVERAGE is set.
3131
32-
NODE_V8_COVERAGE is set by the CLI when coverage is enabled.
33-
Python doesn't use V8 but we use the same env var as the signal.
32+
TUSK_COVERAGE is set by the CLI when coverage is enabled.
33+
This is the language-agnostic signal (Node uses NODE_V8_COVERAGE additionally).
3434
3535
Returns True if coverage was started, False otherwise.
3636
"""
@@ -49,20 +49,29 @@ def start_coverage_collection() -> bool:
4949
)
5050
return False
5151

52-
_source_root = os.getcwd()
53-
54-
_cov_instance = coverage_module.Coverage(
55-
source=[_source_root],
56-
branch=True,
57-
omit=[
58-
"*/site-packages/*",
59-
"*/venv/*",
60-
"*/.venv/*",
61-
"*/test*",
62-
"*/__pycache__/*",
63-
],
64-
)
65-
_cov_instance.start()
52+
with _lock:
53+
# Guard against double initialization — stop existing instance first
54+
if _cov_instance is not None:
55+
try:
56+
_cov_instance.stop()
57+
except Exception:
58+
pass
59+
60+
_source_root = os.path.realpath(os.getcwd())
61+
62+
_cov_instance = coverage_module.Coverage(
63+
source=[_source_root],
64+
branch=True,
65+
omit=[
66+
"*/site-packages/*",
67+
"*/venv/*",
68+
"*/.venv/*",
69+
"*/tests/*",
70+
"*/test_*.py",
71+
"*/__pycache__/*",
72+
],
73+
)
74+
_cov_instance.start()
6675

6776
logger.info("Coverage collection started")
6877
return True
@@ -114,7 +123,8 @@ def take_coverage_snapshot(baseline: bool = False) -> dict:
114123
branch_data = _get_branch_data(data, filename)
115124
if lines_map:
116125
coverage[filename] = {"lines": lines_map, **branch_data}
117-
except Exception:
126+
except Exception as e:
127+
logger.debug(f"Failed to analyze {filename}: {e}")
118128
continue
119129
else:
120130
data = _cov_instance.get_data()
@@ -139,7 +149,10 @@ def _is_user_file(filename: str) -> bool:
139149
"""Check if a file is a user source file (not third-party)."""
140150
if "site-packages" in filename or "lib/python" in filename:
141151
return False
142-
if _source_root and not filename.startswith(_source_root):
152+
# Resolve symlinks for consistent path comparison
153+
resolved = os.path.realpath(filename)
154+
# Use trailing separator to avoid prefix collisions (/app matching /application)
155+
if _source_root and not (resolved.startswith(_source_root + os.sep) or resolved == _source_root):
143156
return False
144157
return True
145158

0 commit comments

Comments
 (0)