Skip to content

Commit 27bb330

Browse files
committed
Test fixes
1 parent 7090042 commit 27bb330

File tree

10 files changed

+594
-599
lines changed

10 files changed

+594
-599
lines changed

TEST_LINTING_PLAN.md

Lines changed: 57 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,51 +22,74 @@
2222

2323
## Staged Approach
2424

25-
### Stage 0: Format All Test Files (Pre-work)
25+
### Stage 0: Format All Test Files (Pre-work) ✅ COMPLETED
2626

2727
**Goal**: Handle all formatting issues upfront before type annotations
2828

2929
**Tasks**:
3030

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
31+
1. Run `ruff format tests/` - 19 files reformatted
32+
2. ✅ Updated configuration to use specific directory exclusions
33+
3. ✅ Verified tests still pass after formatting
3434

3535
**Rationale**: Separating formatting from type annotation work makes it easier to review changes and ensures we start from a clean, consistent base.
3636

37-
**Estimated Effort**: ~15-30 minutes
37+
**Actual Effort**: ~30 minutes
3838

39-
### Stage 1: Infrastructure & Smallest Modules (8 files)
39+
### Stage 1: Infrastructure & Smallest Modules (8 files) ✅ COMPLETED
4040

4141
**Goal**: Validate the approach and establish patterns
4242

4343
**Tasks**:
4444

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
45+
1. ✅ Updated ruff configuration to match pyright (specific directory exclusions)
46+
2. ✅ Ran `ruff check --fix --unsafe-fixes` - auto-fixed 16 errors
47+
3. ✅ Fixed **test_core/test_priorityqueue.py** - 0 errors
48+
4. ✅ Fixed **test_core/test_observer.py** - 0 errors
49+
5. ✅ Fixed **test_core/test_notification.py** - 0 errors (largest file, most complex)
50+
6. ✅ Fixed **test_disposables/test_disposable.py** - 0 errors (auto-fixed)
51+
7. ✅ Fixed **test_testing/test_marbles.py** - 0 errors
52+
8. ✅ All 75 Stage 1 tests pass
53+
54+
**Patterns Discovered**:
55+
56+
- Add `-> None` return types to all test functions
57+
- Use explicit type parameters for generics (e.g., `OnError[int]`, `OnCompleted[int]`)
58+
- Convert string exceptions to `Exception("message")` objects
59+
- Use `cast()` with documented justifications for test patterns that go beyond public API
60+
- Replace `== None` with `is not None` or `is None`
61+
- Add type annotations to all observer classes and helper functions
5162

5263
**Rationale**: These are likely the simplest and will help identify common patterns and issues.
5364

54-
**Estimated Effort**: ~2-4 hours (includes setup)
65+
**Actual Effort**: ~3 hours
5566

56-
### Stage 2: Medium Modules (9 files)
67+
### Stage 2: Medium Modules (9 files) ⚠️ PARTIAL - test_integration COMPLETE
5768

5869
**Goal**: Build confidence with isolated modules
5970

71+
**Status**: test_integration complete, test_subject deferred to preserve commit checkpoint
72+
6073
**Tasks**:
6174

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
75+
1. ⏳ Fix **test_subject** (5 files) - Deferred (requires ~2000 lines of fixes)
76+
2. ✅ Fix **test_integration** (2 files) - COMPLETE
77+
-**test_integration/test_concat_repeat.py** - 0 errors
78+
-**test_integration/test_group_reduce.py** - 0 errors
79+
- ✅ All 3 integration tests pass
80+
3. ✅ Excluded test_subject from pyproject.toml for pre-commit checkpoint
81+
4. ✅ Verified all enabled tests pass (78 tests total)
6682

67-
**Rationale**: Still manageable size, builds confidence before tackling large modules.
83+
**Additional Patterns Discovered**:
6884

69-
**Estimated Effort**: ~1-2 hours
85+
- Marble testing requires `None, None` parameters for value_lookup and error_lookup
86+
- Complex operator chains (group_by, flat_map): Extract helper function with explicit `Callable[[Observable[Any]], Observable[Any]]` type
87+
- Integration tests validate runtime behavior, so `Any` types with documented justification are acceptable
88+
- GroupedObservable type inference issues: Use helper functions instead of inline lambdas
89+
90+
**Rationale**: Still manageable size, builds confidence before tackling large modules. Split to allow checkpoint commit.
91+
92+
**Actual Effort**: ~1 hour (test_integration only)
7093

7194
### Stage 3: Scheduler Module (27 files)
7295

@@ -271,21 +294,23 @@ typeCheckingMode = "strict"
271294
- [x] Run `ruff format tests/` (19 files reformatted)
272295
- [x] Review and commit formatting changes (commit: 73495f83)
273296
- [x] Verify tests still pass
274-
- [x] Update ruff config to use specific directory exclusions (commit: pending)
297+
- [x] Update ruff config to use specific directory exclusions (commit: ecf450ee)
275298

276299
**Note**: Formatting worked even with blanket "tests" exclusion because explicitly passing paths to ruff overrides excludes by default.
277300

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
301+
- [x] Stage 1: Infrastructure & Smallest Modules (8 files) ✅
302+
- [x] Update configuration files
303+
- [x] Fix test_core (4 files) - 0 errors
304+
- [x] Fix test_disposables (2 files) - 0 errors
305+
- [x] Fix test_testing (2 files) - 0 errors
306+
- [x] Document patterns
307+
- [x] All 75 tests pass
308+
309+
- [x] Stage 2: Medium Modules (9 files) ⚠️ PARTIAL
310+
- [ ] Fix test_subject (5 files) - Deferred
311+
- [x] Fix test_integration (2 files) - 0 errors, 3 tests pass
312+
- [x] Exclude test_subject from pyproject.toml
313+
- [x] Run enabled tests (78 tests total)
289314

290315
- [ ] Stage 3: Scheduler Module (27 files)
291316
- [ ] Fix test_scheduler files

pyproject.toml

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,8 @@ exclude = [
7575
"notebooks",
7676
"examples",
7777
# Test directories - remove as each stage is completed
78-
# Stage 1 directories removed - now enabled for linting
78+
# Stage 1 complete, Stage 2 partial (test_integration complete)
7979
"tests/test_subject",
80-
"tests/test_integration",
8180
"tests/test_scheduler",
8281
"tests/test_observable",
8382
]
@@ -101,7 +100,6 @@ known-first-party = ["reactivex"]
101100
python_version = "3.10"
102101
follow_imports = "silent"
103102
files = ["reactivex"]
104-
exclude = ["reactivex/operators/_\\w.*\\.py$"] # mypy will eventually catch up
105103
disallow_any_generics = true
106104
disallow_untyped_defs = true
107105

@@ -112,9 +110,8 @@ asyncio_mode = "strict"
112110
[tool.pyright]
113111
include = ["reactivex", "tests"]
114112
exclude = [
115-
# Stage 1 directories removed - now enabled for type checking
113+
# Stage 1 complete, Stage 2 partial (test_integration complete)
116114
"tests/test_subject",
117-
"tests/test_integration",
118115
"tests/test_scheduler",
119116
"tests/test_observable",
120117
]

reactivex/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def amb(*sources: Observable[_T]) -> Observable[_T]:
6464
def case(
6565
mapper: Callable[[], _TKey],
6666
sources: Mapping[_TKey, Observable[_T]],
67-
default_source: Union[Observable[_T], "Future[_T]"] | None = None,
67+
default_source: Observable[_T] | Future[_T] | None = None,
6868
) -> Observable[_T]:
6969
"""Uses mapper to determine which source in sources to use.
7070

tests/test_integration/test_concat_repeat.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55

66

77
class TestConcatIntegration(unittest.TestCase):
8-
def test_concat_repeat(self):
9-
with marbles_testing() as (start, cold, hot, exp):
10-
e1 = cold("-e11-e12|")
11-
e2 = cold("-e21-e22|")
12-
ex = exp("-e11-e12-e21-e22-e11-e12-e21-e22|")
8+
def test_concat_repeat(self) -> None:
9+
with marbles_testing() as (start, cold, _hot, exp):
10+
e1 = cold("-e11-e12|", None, None)
11+
e2 = cold("-e21-e22|", None, None)
12+
ex = exp("-e11-e12-e21-e22-e11-e12-e21-e22|", None, None)
1313

1414
obs = e1.pipe(ops.concat(e2), ops.repeat(2))
1515

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,50 @@
11
import unittest
2+
from collections.abc import Callable
3+
from typing import Any
24

35
import reactivex
6+
from reactivex import Observable
47
from reactivex import operators as ops
58

69

710
class TestGroupByReduce(unittest.TestCase):
8-
def test_groupby_count(self):
9-
res = []
10-
counts = reactivex.from_(range(10)).pipe(
11+
def test_groupby_count(self) -> None:
12+
res: list[Any] = []
13+
14+
# Integration test: GroupedObservable types validated at runtime
15+
def flat_map_fn(i: Any) -> Any:
16+
return i.pipe(
17+
ops.count(),
18+
ops.map(lambda ii: (i.key, ii)),
19+
)
20+
21+
flat_map_op: Callable[[Observable[Any]], Observable[Any]] = ops.flat_map(
22+
flat_map_fn
23+
)
24+
25+
counts: Any = reactivex.from_(range(10)).pipe(
1126
ops.group_by(lambda i: "even" if i % 2 == 0 else "odd"),
12-
ops.flat_map(
13-
lambda i: i.pipe(
14-
ops.count(),
15-
ops.map(lambda ii: (i.key, ii)),
16-
)
17-
),
27+
flat_map_op,
1828
)
1929

2030
counts.subscribe(on_next=res.append)
2131
assert res == [("even", 5), ("odd", 5)]
2232

23-
def test_window_sum(self):
24-
res = []
25-
reactivex.from_(range(6)).pipe(
33+
def test_window_sum(self) -> None:
34+
res: list[Any] = []
35+
36+
# Integration test: windowed Observable types validated at runtime
37+
def flat_map_fn(i: Any) -> Any:
38+
return i.pipe(ops.sum())
39+
40+
flat_map_op: Callable[[Observable[Any]], Observable[Any]] = ops.flat_map(
41+
flat_map_fn
42+
)
43+
44+
obs: Any = reactivex.from_(range(6)).pipe(
2645
ops.window_with_count(count=3, skip=1),
27-
ops.flat_map(
28-
lambda i: i.pipe(
29-
ops.sum(),
30-
)
31-
),
32-
).subscribe(on_next=res.append)
46+
flat_map_op,
47+
)
48+
obs.subscribe(on_next=res.append)
3349

3450
assert res == [3, 6, 9, 12, 9, 5, 0]

0 commit comments

Comments
 (0)