Commit 68c628b
authored
Hierarchical symbol impact analysis for pytest marker analyzer (#3968)
## Summary
Adds member-level and function-call narrowing to the pytest marker
analyzer to dramatically reduce false positives when determining which
marked tests should run based on PR changes.
### Problem
The analyzer treated entire classes as single symbols. When ANY method
inside a ~1000-line class like `VirtualMachineForTests` changed, ALL
tests importing the class were flagged as affected — even tests that
never use the modified methods. PR #3847 reported 83 affected smoke
tests when the actual count was 2.
### Solution
Three layers of narrowing:
1. **Member-level narrowing** — Maps changed lines to specific class
members (methods/properties). Builds an intra-class call graph to expand
transitively (if method A calls self.B() and B is modified, A is also
affected).
2. **Function-call narrowing** — Checks if the specific test function
actually calls the modified top-level functions, rather than relying on
file-level imports. This handles the case where a test file imports
`assert_vm_xml_efi` for sibling test functions, but the smoke tests in
the same file never call it.
3. **Test file self-modification** — When a test's own file is changed,
applies symbol analysis instead of blindly flagging. Only flags the test
if its own function/class was modified.
### Additional improvements
- **Remote mode fix**: Fetches the PR's HEAD version of files via GitHub
API so diff line numbers align correctly with symbol maps (previously
used local file which could be on a different branch)
- **Code deduplication**: Consolidated 4 duplicate code paths
(`_is_fixture_decorator`, `_parse_diff_for_functions`,
`_get_modified_function_names`, `extract_modified_items`) into
standalone module-level functions
- **Extracted helpers**: `_parse_test_name()`,
`_find_test_function_node()` to eliminate repeated patterns
### Results
| PR | Before | After | Reduction |
|----|--------|-------|-----------|
| #3847 (VirtualMachineForTests refactor, 69 files) | 83/83 smoke tests
| 2/83 smoke tests | **97.6%** |
The 2 remaining tests (`test_migrate_vm` variants) genuinely call
`validate_libvirt_persistent_domain` whose signature changed.
### Safety invariants preserved
| Scenario | Behavior |
|---|---|
| Changed line inside class but outside any method | Class-level
fallback (flags all importers) |
| `getattr()`/`setattr()`/`delattr()` in test body | Returns None → no
narrowing (conservative) |
| Method removed from class | Included as modified → tests flagged |
| Test imports class but usage can't be determined | No narrowing
(conservative) |
| `__init__` modified + test constructs class | Always included in
accessed attrs |
| Remote mode API fetch fails | Returns None → file-level fallback |
Co-authored-by: Claude <noreply@anthropic.com>
## Test plan
- [x] `pre-commit run --all-files` — all 15 hooks pass
- [x] `tox -e utilities-unittests` — 671 tests pass, 98.61% coverage
- [x] `uv run pytest scripts/tests_analyzer/tests/ -v` — 56 new tests
pass
- [x] Validated against PR #3847: 83→2 affected smoke tests
- [x] Validated against 9 mismatch PRs from CodeRabbit comparison
- [x] Code review loop: 3 reviewers, all findings addressed1 parent a2da77f commit 68c628b
File tree
6 files changed
+2477
-619
lines changed- scripts/tests_analyzer
- tests
6 files changed
+2477
-619
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| 17 | + | |
17 | 18 | | |
18 | 19 | | |
19 | 20 | | |
| |||
0 commit comments