|
| 1 | +# Multi-Stage Plan: Enable Linting & Type Checking for Tests |
| 2 | + |
| 3 | +## Current Situation |
| 4 | + |
| 5 | +- **184 test files** currently excluded from ruff and pyright |
| 6 | +- Tests are excluded in both `pyrightconfig.json` and `pyproject.toml` |
| 7 | +- Largest category: **test_observable (142 files)** - 77% of all tests |
| 8 | +- Project uses **strict type checking** standards (pyright --strict, mypy --strict) |
| 9 | + |
| 10 | +## Test File Distribution |
| 11 | + |
| 12 | +| Directory | File Count | Percentage | |
| 13 | +|-----------|------------|------------| |
| 14 | +| test_observable | 142 | 77% | |
| 15 | +| test_scheduler | 27 | 15% | |
| 16 | +| test_subject | 5 | 3% | |
| 17 | +| test_core | 4 | 2% | |
| 18 | +| test_disposables | 2 | 1% | |
| 19 | +| test_integration | 2 | 1% | |
| 20 | +| test_testing | 2 | 1% | |
| 21 | +| **Total** | **184** | **100%** | |
| 22 | + |
| 23 | +## Staged Approach |
| 24 | + |
| 25 | +### Stage 0: Format All Test Files (Pre-work) |
| 26 | + |
| 27 | +**Goal**: Handle all formatting issues upfront before type annotations |
| 28 | + |
| 29 | +**Tasks**: |
| 30 | + |
| 31 | +1. Run `ruff format tests/` to auto-format all test files |
| 32 | +2. Review and commit formatting changes |
| 33 | +3. Verify tests still pass after formatting |
| 34 | + |
| 35 | +**Rationale**: Separating formatting from type annotation work makes it easier to review changes and ensures we start from a clean, consistent base. |
| 36 | + |
| 37 | +**Estimated Effort**: ~15-30 minutes |
| 38 | + |
| 39 | +### Stage 1: Infrastructure & Smallest Modules (8 files) |
| 40 | + |
| 41 | +**Goal**: Validate the approach and establish patterns |
| 42 | + |
| 43 | +**Tasks**: |
| 44 | + |
| 45 | +1. ✅ **COMPLETED**: Updated ruff configuration to match pyright (specific directory exclusions) |
| 46 | +2. Run `ruff check --fix` on Stage 1 files for auto-fixable issues |
| 47 | +3. Fix **test_core** (4 files) - Core functionality tests |
| 48 | +4. Fix **test_disposables** (2 files) - Disposable tests |
| 49 | +5. Fix **test_testing** (2 files) - Testing utilities tests |
| 50 | +6. Document common patterns and solutions |
| 51 | + |
| 52 | +**Rationale**: These are likely the simplest and will help identify common patterns and issues. |
| 53 | + |
| 54 | +**Estimated Effort**: ~2-4 hours (includes setup) |
| 55 | + |
| 56 | +### Stage 2: Medium Modules (9 files) |
| 57 | + |
| 58 | +**Goal**: Build confidence with isolated modules |
| 59 | + |
| 60 | +**Tasks**: |
| 61 | + |
| 62 | +1. Fix **test_subject** (5 files) - Subject tests |
| 63 | +2. Fix **test_integration** (2 files) - Integration tests |
| 64 | +3. Run full test suite to ensure no regressions |
| 65 | +4. Update pattern documentation |
| 66 | + |
| 67 | +**Rationale**: Still manageable size, builds confidence before tackling large modules. |
| 68 | + |
| 69 | +**Estimated Effort**: ~1-2 hours |
| 70 | + |
| 71 | +### Stage 3: Scheduler Module (27 files) |
| 72 | + |
| 73 | +**Goal**: Tackle a substantial module with known complexity |
| 74 | + |
| 75 | +**Tasks**: |
| 76 | +Fix **test_scheduler** (27 files) - May need to be sub-batched: |
| 77 | + |
| 78 | +- test_scheduler/test_eventloop (AsyncIO schedulers) |
| 79 | +- test_scheduler/test_currentthread |
| 80 | +- test_scheduler/test_historicalscheduler |
| 81 | +- test_scheduler/test_timeout |
| 82 | +- Other scheduler tests |
| 83 | + |
| 84 | +**Rationale**: Isolated module with clear boundaries, but large enough to benefit from batching. |
| 85 | + |
| 86 | +**Estimated Effort**: ~3-5 hours |
| 87 | + |
| 88 | +### Stage 4: Observable Module - Batched (142 files) |
| 89 | + |
| 90 | +**Goal**: Systematically fix the largest test suite by operator category |
| 91 | + |
| 92 | +The massive observable tests need careful batching by operator category to align with the mixin architecture: |
| 93 | + |
| 94 | +#### Batch 4a: Filtering Operators (~20 files) |
| 95 | + |
| 96 | +- filter, take, skip, take_while, skip_while |
| 97 | +- distinct, distinct_until_changed |
| 98 | +- element_at, first, last, sample, throttle |
| 99 | + |
| 100 | +**Estimated Effort**: ~2-3 hours |
| 101 | + |
| 102 | +#### Batch 4b: Transformation Operators (~25 files) |
| 103 | + |
| 104 | +- map, flat_map, flat_map_indexed, flat_map_latest |
| 105 | +- scan, reduce, expand |
| 106 | +- pluck, starmap, switch_map |
| 107 | + |
| 108 | +**Estimated Effort**: ~3-4 hours |
| 109 | + |
| 110 | +#### Batch 4c: Combination Operators (~20 files) |
| 111 | + |
| 112 | +- merge, concat, zip, zip_with_iterable |
| 113 | +- combine_latest, with_latest_from |
| 114 | +- start_with, concat_all, merge_all |
| 115 | + |
| 116 | +**Estimated Effort**: ~2-3 hours |
| 117 | + |
| 118 | +#### Batch 4d: Time-Based Operators (~15 files) |
| 119 | + |
| 120 | +- debounce, throttle, sample, delay |
| 121 | +- timeout, interval, timer |
| 122 | +- timestamp, time_interval |
| 123 | + |
| 124 | +**Estimated Effort**: ~2-3 hours |
| 125 | + |
| 126 | +#### Batch 4e: Mathematical Operators (~10 files) |
| 127 | + |
| 128 | +- count, sum, average, min, max |
| 129 | +- reduce (if not covered in 4b) |
| 130 | + |
| 131 | +**Estimated Effort**: ~1-2 hours |
| 132 | + |
| 133 | +#### Batch 4f: Error Handling Operators (~10 files) |
| 134 | + |
| 135 | +- catch, retry, on_error_resume_next |
| 136 | +- catch_with_iterable |
| 137 | + |
| 138 | +**Estimated Effort**: ~1-2 hours |
| 139 | + |
| 140 | +#### Batch 4g: Utility Operators (~15 files) |
| 141 | + |
| 142 | +- do_action, do, tap |
| 143 | +- materialize, dematerialize |
| 144 | +- observe_on, subscribe_on |
| 145 | +- timestamp, timeout |
| 146 | + |
| 147 | +**Estimated Effort**: ~2-3 hours |
| 148 | + |
| 149 | +#### Batch 4h: Windowing Operators (~12 files) |
| 150 | + |
| 151 | +- buffer, buffer_with_count, buffer_with_time |
| 152 | +- window, window_with_count, window_with_time |
| 153 | +- group_by, partition |
| 154 | + |
| 155 | +**Estimated Effort**: ~2-3 hours |
| 156 | + |
| 157 | +#### Batch 4i: Remaining Operators (~15 files) |
| 158 | + |
| 159 | +- All other observable tests not covered above |
| 160 | +- share, publish, replay, ref_count |
| 161 | +- to_list, to_dict, to_set |
| 162 | +- contains, sequence_equal, default_if_empty |
| 163 | + |
| 164 | +**Estimated Effort**: ~2-3 hours |
| 165 | + |
| 166 | +## Configuration Strategy |
| 167 | + |
| 168 | +**All configuration is consolidated in `pyproject.toml`** - no separate pyright config file needed. |
| 169 | + |
| 170 | +### pyproject.toml |
| 171 | + |
| 172 | +Use granular exclude patterns that get removed as files are fixed: |
| 173 | + |
| 174 | +```toml |
| 175 | +[tool.ruff] |
| 176 | +line-length = 88 |
| 177 | +target-version = "py310" |
| 178 | +exclude = [ |
| 179 | + ".git", |
| 180 | + "__pycache__", |
| 181 | + "docs/source/conf.py", |
| 182 | + "old", |
| 183 | + "build", |
| 184 | + "dist", |
| 185 | + "notebooks", |
| 186 | + "examples", |
| 187 | + # Stage 1-3: Remove these directory exclusions as fixed |
| 188 | + "tests/test_core", |
| 189 | + "tests/test_disposables", |
| 190 | + "tests/test_testing", |
| 191 | + "tests/test_subject", |
| 192 | + "tests/test_integration", |
| 193 | + "tests/test_scheduler", |
| 194 | + # Stage 4: For test_observable, exclude individual files |
| 195 | + # Remove files from this list as batches are completed |
| 196 | + "tests/test_observable/test_filter.py", |
| 197 | + "tests/test_observable/test_map.py", |
| 198 | + # ... etc (add all 142 files, remove as fixed) |
| 199 | +] |
| 200 | + |
| 201 | +[tool.pyright] |
| 202 | +include = ["reactivex", "tests"] |
| 203 | +exclude = [ |
| 204 | + # Stage 1-3: Remove these directory exclusions as fixed |
| 205 | + "tests/test_core", |
| 206 | + "tests/test_disposables", |
| 207 | + "tests/test_testing", |
| 208 | + "tests/test_subject", |
| 209 | + "tests/test_integration", |
| 210 | + "tests/test_scheduler", |
| 211 | + # Stage 4: For test_observable, exclude individual files |
| 212 | + # Remove files from this list as batches are completed |
| 213 | + "tests/test_observable/test_filter.py", |
| 214 | + "tests/test_observable/test_map.py", |
| 215 | + # ... etc (add all 142 files, remove as fixed) |
| 216 | +] |
| 217 | +reportImportCycles = false |
| 218 | +reportMissingImports = false |
| 219 | +pythonVersion = "3.10" |
| 220 | +typeCheckingMode = "strict" |
| 221 | +``` |
| 222 | + |
| 223 | +## Common Issues to Address Per Batch |
| 224 | + |
| 225 | +1. **Missing type annotations** on test methods and fixtures |
| 226 | + - Add return types to test methods (`-> None`) |
| 227 | + - Type test helper functions |
| 228 | + - Type lambda parameters |
| 229 | + |
| 230 | +2. **Any types** from operators without proper casts |
| 231 | + - Use documented `cast` with justifications |
| 232 | + - Preserve type parameters through operator chains |
| 233 | + |
| 234 | +3. **Untyped test helpers** and utility functions |
| 235 | + - Create typed wrappers or add annotations |
| 236 | + - Use generics where appropriate |
| 237 | + |
| 238 | +4. **Import organization** (isort/ruff formatting) |
| 239 | + - Ensure imports are sorted correctly |
| 240 | + - Group imports properly (stdlib, third-party, first-party) |
| 241 | + |
| 242 | +5. **TestScheduler type safety** - Often uses `Any` for message values |
| 243 | + - Use proper type parameters for `ReactiveTest.on_next(time, value)` |
| 244 | + - Consider `Observable[Any]` where message types vary |
| 245 | + |
| 246 | +6. **Observer/Observable type parameters** - Need explicit type vars |
| 247 | + - Ensure `Observable[T]` is properly typed throughout tests |
| 248 | + - Use `TypeVar` for generic test helpers |
| 249 | + |
| 250 | +7. **Deprecated APIs** - Update any deprecated datetime usage |
| 251 | + - Replace `datetime.utcfromtimestamp()` with timezone-aware versions |
| 252 | + - Use `default_now()` where appropriate |
| 253 | + |
| 254 | +8. **Test class inheritance** |
| 255 | + - Ensure `unittest.TestCase` inheritance is properly typed |
| 256 | + - Type `setUp` and `tearDown` methods |
| 257 | + |
| 258 | +## Safety Measures |
| 259 | + |
| 260 | +- ✅ Run full test suite after each stage (`uv run pytest`) |
| 261 | +- ✅ Keep changes focused on type annotations and linting fixes only |
| 262 | +- ✅ Don't change test logic or behavior |
| 263 | +- ✅ Use documented `cast` with justifications when needed |
| 264 | +- ✅ Run pre-commit hooks before committing (`uv run pre-commit run --all-files`) |
| 265 | +- ✅ Commit after each successful batch with clear commit messages |
| 266 | +- ✅ Verify pyright and ruff pass on modified files before proceeding |
| 267 | + |
| 268 | +## Progress Tracking |
| 269 | + |
| 270 | +- [x] Stage 0: Format All Test Files (Pre-work) ✅ |
| 271 | + - [x] Run `ruff format tests/` (19 files reformatted) |
| 272 | + - [x] Review and commit formatting changes (commit: 73495f83) |
| 273 | + - [x] Verify tests still pass |
| 274 | + - [x] Update ruff config to use specific directory exclusions (commit: pending) |
| 275 | + |
| 276 | + **Note**: Formatting worked even with blanket "tests" exclusion because explicitly passing paths to ruff overrides excludes by default. |
| 277 | + |
| 278 | +- [ ] Stage 1: Infrastructure & Smallest Modules (8 files) |
| 279 | + - [ ] Update configuration files |
| 280 | + - [ ] Fix test_core (4 files) |
| 281 | + - [ ] Fix test_disposables (2 files) |
| 282 | + - [ ] Fix test_testing (2 files) |
| 283 | + - [ ] Document patterns |
| 284 | + |
| 285 | +- [ ] Stage 2: Medium Modules (9 files) |
| 286 | + - [ ] Fix test_subject (5 files) |
| 287 | + - [ ] Fix test_integration (2 files) |
| 288 | + - [ ] Run full test suite |
| 289 | + |
| 290 | +- [ ] Stage 3: Scheduler Module (27 files) |
| 291 | + - [ ] Fix test_scheduler files |
| 292 | + - [ ] Run full test suite |
| 293 | + |
| 294 | +- [ ] Stage 4: Observable Module (142 files) |
| 295 | + - [ ] Batch 4a: Filtering operators (~20 files) |
| 296 | + - [ ] Batch 4b: Transformation operators (~25 files) |
| 297 | + - [ ] Batch 4c: Combination operators (~20 files) |
| 298 | + - [ ] Batch 4d: Time-based operators (~15 files) |
| 299 | + - [ ] Batch 4e: Mathematical operators (~10 files) |
| 300 | + - [ ] Batch 4f: Error handling operators (~10 files) |
| 301 | + - [ ] Batch 4g: Utility operators (~15 files) |
| 302 | + - [ ] Batch 4h: Windowing operators (~12 files) |
| 303 | + - [ ] Batch 4i: Remaining operators (~15 files) |
| 304 | + |
| 305 | +- [ ] Final: Complete cleanup |
| 306 | + - [ ] Remove all test exclusions from config files |
| 307 | + - [ ] Create TESTING_TYPES.md documentation |
| 308 | + - [ ] Update CLAUDE.md with testing type patterns |
| 309 | + - [ ] Final full test suite run |
| 310 | + - [ ] Final pre-commit check |
| 311 | + |
| 312 | +## Pattern Documentation |
| 313 | + |
| 314 | +As patterns emerge during Stage 1, document them here or in a separate `TESTING_TYPES.md` file: |
| 315 | + |
| 316 | +### Common Pattern 1: Type Test Methods |
| 317 | + |
| 318 | +```python |
| 319 | +def test_something(self) -> None: |
| 320 | + """Test description.""" |
| 321 | + # Test implementation |
| 322 | +``` |
| 323 | + |
| 324 | +### Common Pattern 2: Typed TestScheduler Usage |
| 325 | + |
| 326 | +```python |
| 327 | +def test_operator(self) -> None: |
| 328 | + scheduler = TestScheduler() |
| 329 | + |
| 330 | + xs: Observable[int] = scheduler.create_hot_observable( |
| 331 | + ReactiveTest.on_next(210, 1), |
| 332 | + ReactiveTest.on_next(220, 2), |
| 333 | + ReactiveTest.on_completed(250) |
| 334 | + ) |
| 335 | + |
| 336 | + result = scheduler.start(lambda: xs.pipe(ops.map(lambda x: x * 2))) |
| 337 | +``` |
| 338 | + |
| 339 | +### Common Pattern 3: Cast for Complex Operators |
| 340 | + |
| 341 | +```python |
| 342 | +from collections.abc import Callable |
| 343 | +from typing import Any, cast |
| 344 | + |
| 345 | +def test_complex_operator(self) -> None: |
| 346 | + source: Observable[int] = # ... |
| 347 | + |
| 348 | + # When operator type inference fails |
| 349 | + op: Callable[[Observable[int]], Observable[str]] = cast( |
| 350 | + "Callable[[Observable[int]], Observable[str]]", |
| 351 | + ops.map(lambda x: str(x)) |
| 352 | + # Cast is safe because map preserves the transformation type |
| 353 | + ) |
| 354 | + result = source.pipe(op) |
| 355 | +``` |
| 356 | + |
| 357 | +## Estimated Total Effort |
| 358 | + |
| 359 | +- **Stage 0**: ~15-30 minutes (formatting) |
| 360 | +- **Stage 1**: ~2-4 hours (includes setup and pattern discovery) |
| 361 | +- **Stage 2**: ~1-2 hours |
| 362 | +- **Stage 3**: ~3-5 hours |
| 363 | +- **Stage 4**: ~15-25 hours (across 9 batches) |
| 364 | +- **Final cleanup**: ~1-2 hours |
| 365 | +- **Total**: ~22.5-38.5 hours |
| 366 | + |
| 367 | +## Success Criteria |
| 368 | + |
| 369 | +1. ✅ All test files pass `pyright --strict` with zero errors/warnings |
| 370 | +2. ✅ All test files pass `ruff check` with zero errors |
| 371 | +3. ✅ All test files pass `ruff format --check` (properly formatted) |
| 372 | +4. ✅ Full test suite passes (`uv run pytest`) |
| 373 | +5. ✅ Pre-commit hooks pass on all test files |
| 374 | +6. ✅ No `type: ignore` comments (use documented `cast` instead) |
| 375 | +7. ✅ Comprehensive pattern documentation exists for future test development |
| 376 | + |
| 377 | +## Notes |
| 378 | + |
| 379 | +- This plan aligns with RxPY's strict type safety standards |
| 380 | +- Breaking work into small batches allows for incremental progress |
| 381 | +- Each stage can be completed and committed independently |
| 382 | +- If a batch proves too large, it can be further subdivided |
| 383 | +- Pattern documentation will help maintain type safety in future tests |
0 commit comments