Fix/dedup without ext lib #15
Conversation
Signed-off-by: Asish <officialasishkumar@gmail.com>
Signed-off-by: Asish <officialasishkumar@gmail.com>
Signed-off-by: Asish <officialasishkumar@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the previous coverage.py-based implementation with a custom Python tracer solution to eliminate external library dependencies while providing deduplication and per-test coverage tracking.
- Replaces coverage.py with Python's built-in
tracemodule for coverage collection - Implements always-on tracing with per-test differential reporting
- Adds thread synchronization logic to wait for worker threads to complete before reporting
| _tracer.results().counts.clear() # start from 0 hits | ||
| baseline_counts = {} # diff against empty dict | ||
| baseline_tids = _current_tids() # remember threads |
There was a problem hiding this comment.
Clearing the global tracer counts without synchronization could cause race conditions if multiple threads are executing concurrently. The tracer is shared across all threads but this operation is not atomic.
| _tracer.results().counts.clear() # start from 0 hits | |
| baseline_counts = {} # diff against empty dict | |
| baseline_tids = _current_tids() # remember threads | |
| with lock: | |
| _tracer.results().counts.clear() # start from 0 hits | |
| baseline_counts = {} # diff against empty dict | |
| baseline_tids = _current_tids() # remember threads |
Copilot uses AI. Check for mistakes.
| def _copy_counts_once() -> dict[tuple[str, int], int]: | ||
| while True: | ||
| try: return dict(_tracer.results().counts) | ||
| except RuntimeError: continue # mutated – retry |
There was a problem hiding this comment.
The infinite retry loop without any delay or maximum attempt limit could lead to excessive CPU usage if the RuntimeError persists. Consider adding a small delay and maximum retry count.
| def _copy_counts_once() -> dict[tuple[str, int], int]: | |
| while True: | |
| try: return dict(_tracer.results().counts) | |
| except RuntimeError: continue # mutated – retry | |
| def _copy_counts_once(max_retries: int = 100, retry_delay: float = 0.05) -> dict[tuple[str, int], int]: | |
| retries = 0 | |
| while retries < max_retries: | |
| try: | |
| return dict(_tracer.results().counts) | |
| except RuntimeError: | |
| retries += 1 | |
| time.sleep(retry_delay) # Introduce a small delay | |
| raise RuntimeError("Failed to copy counts after maximum retries") |
Copilot uses AI. Check for mistakes.
| if action == "END": | ||
| logging.info(f"END {test_id}") | ||
| if test_id == current_id: | ||
| _wait_for_worker_exit(baseline_tids) # <- NEW |
There was a problem hiding this comment.
[nitpick] The comment '# <- NEW' is not informative and adds no value to code understanding. Consider removing it or replacing with a more descriptive comment explaining why this wait is necessary.
| _wait_for_worker_exit(baseline_tids) # <- NEW | |
| _wait_for_worker_exit(baseline_tids) # Ensure no extra threads remain before emitting results |
Copilot uses AI. Check for mistakes.
| logging.info(f"END {test_id}") | ||
| if test_id == current_id: | ||
| _wait_for_worker_exit(baseline_tids) # <- NEW | ||
| time.sleep(0.02) |
There was a problem hiding this comment.
The magic number 0.02 for sleep duration should be defined as a named constant to improve code maintainability and make the timing requirement explicit.
| time.sleep(0.02) | |
| time.sleep(SLEEP_DURATION) |
Copilot uses AI. Check for mistakes.
| def _stable_snapshot(max_wait: float = .5) -> dict[tuple[str, int], int]: | ||
| start = time.time(); snap = _copy_counts_once() | ||
| while True: | ||
| time.sleep(0.07) |
There was a problem hiding this comment.
The magic number 0.07 for sleep duration should be defined as a named constant to improve code maintainability and make the polling interval explicit.
| time.sleep(0.07) | |
| time.sleep(POLLING_INTERVAL) |
Copilot uses AI. Check for mistakes.
| while time.time() < deadline: | ||
| if _current_tids() <= baseline: # no extra threads left | ||
| return | ||
| time.sleep(0.02) # wait 20 ms and retry |
There was a problem hiding this comment.
The magic number 0.02 for sleep duration should be defined as a named constant to improve code maintainability and consistency with other timing values.
| time.sleep(0.02) # wait 20 ms and retry | |
| time.sleep(WORKER_EXIT_SLEEP_INTERVAL) # wait 20 ms and retry |
Copilot uses AI. Check for mistakes.
Issue that this pull request solves
Closes: # (issue number)
Proposed changes
Brief description of what is fixed or changed
Types of changes
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that applyOther information
Any other information that is important to this pull request