Skip to content

Commit e01c04b

Browse files
jeremyederjeremyeder
authored
Merge 23ac90e into 8243fd8 
2 parents 8243fd8 + 23ac90e commit e01c04b

File tree

3 files changed

+228
-9
lines changed

3 files changed

+228
-9
lines changed

‎.github/workflows/update-leaderboard.yml‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,9 @@ jobs:
3535
if [ -n "$(git status --porcelain docs/_data/leaderboard.json)" ]; then
3636
git add docs/_data/leaderboard.json
3737
38-
# Use static commit message (no user input)
3938
TIMESTAMP=$(date -u '+%Y-%m-%d %H:%M:%S UTC')
40-
git commit -m "chore: update leaderboard data [skip ci]
41-
42-
Generated from submissions/ directory at $TIMESTAMP"
39+
git commit -m "chore: update leaderboard data [skip ci]" \
40+
-m "Generated from submissions/ directory at $TIMESTAMP"
4341
git push
4442
else
4543
echo "No changes to leaderboard data"

‎specs/test-suite-fixes.md‎

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
# Test Suite Fixes - Cold Start Prompt
2+
3+
## Context
4+
5+
After merging PR #146 (Leaderboard feature), the CI shows 71 test failures. These are **pre-existing failures** unrelated to the leaderboard implementation, but they need to be fixed to maintain code quality and CI reliability.
6+
7+
## Test Failure Categories
8+
9+
### 1. Config Validation Tests (7 failures)
10+
**Files**: `tests/unit/cli/test_main.py::TestConfigLoading`
11+
12+
**Issues**:
13+
- `test_load_config_not_dict` - TypeError with Config() constructor
14+
- `test_load_config_unknown_keys` - Not raising expected ValueError
15+
- `test_load_config_invalid_weights_type` - SystemExit instead of validation
16+
- `test_load_config_invalid_weight_value` - SystemExit instead of validation
17+
- `test_load_config_invalid_excluded_attributes` - SystemExit instead of validation
18+
- `test_load_config_sensitive_output_dir` - SystemExit instead of validation
19+
- `test_load_config_invalid_report_theme` - SystemExit instead of validation
20+
21+
**Root Cause**: Config validation logic moved to Click callback validators, but tests expect model-level validation
22+
23+
**Fix Strategy**:
24+
- Update tests to expect Click validation errors (SystemExit)
25+
- Or add validation to Config model `__post_init__`
26+
- Ensure consistency between CLI and model validation
27+
28+
### 2. Assessment Fixture Validation (15+ failures)
29+
**Files**: Multiple test files creating mock Assessment objects
30+
31+
**Issues**:
32+
- `ValueError: Findings count (0) must equal attributes total (1)`
33+
- Mock assessments created without findings
34+
- New validation added in `Assessment.validate()` enforces findings count
35+
36+
**Root Cause**: Assessment model now validates that `len(findings) == attributes_total`
37+
38+
**Fix Strategy**:
39+
- Update all test fixtures to include proper findings array
40+
- Or create a test-only Assessment factory that bypasses validation
41+
- Example pattern:
42+
```python
43+
# Create valid mock finding
44+
finding = Finding.create_pass(
45+
attribute=mock_attribute,
46+
evidence=["Test evidence"]
47+
)
48+
49+
# Include in assessment
50+
assessment = Assessment(
51+
repository=mock_repo,
52+
findings=[finding],
53+
attributes_total=1,
54+
# ...
55+
)
56+
```
57+
58+
### 3. CLI Align Command Tests (12 failures)
59+
**Files**: `tests/unit/test_cli_align.py`
60+
61+
**Issue**: `AttributeError: <module 'agentready.cli.align'> does not have the attribute 'LanguageDetector'`
62+
63+
**Root Cause**: Tests trying to mock `align.LanguageDetector` but it's imported from `services.language_detector`
64+
65+
**Fix Strategy**:
66+
- Update mocks to patch correct import path: `@patch('agentready.services.language_detector.LanguageDetector')`
67+
- Or patch at usage site: `@patch('agentready.cli.align.LanguageDetector')` after ensuring import is at module level
68+
69+
### 4. Learning Service Tests (9 failures)
70+
**Files**: `tests/unit/test_learning_service.py`
71+
72+
**Issue**: `ValueError: Not a git repository: /tmp/...`
73+
74+
**Root Cause**: LearningService expects git repository but test fixtures use plain temp directories
75+
76+
**Fix Strategy**:
77+
- Initialize git repo in test fixtures:
78+
```python
79+
@pytest.fixture
80+
def mock_repo(tmp_path):
81+
# Create .git directory
82+
(tmp_path / ".git").mkdir()
83+
subprocess.run(["git", "init"], cwd=tmp_path, check=True)
84+
return tmp_path
85+
```
86+
87+
### 5. Pattern Extractor Tests (8 failures)
88+
**Files**: `tests/unit/learners/test_pattern_extractor.py`
89+
90+
**Issue**: `AttributeError: 'Attribute' object has no attribute 'attribute_id'`
91+
92+
**Root Cause**: Attribute model uses `id` not `attribute_id`, but pattern extractor expects old field name
93+
94+
**Fix Strategy**:
95+
- Update PatternExtractor to use `attribute.id` instead of `attribute.attribute_id`
96+
- Check all code for this pattern
97+
98+
### 6. GitHub Scanner Tests (5 failures)
99+
**Files**: `tests/unit/test_github_scanner.py`
100+
101+
**Issue**: `AssertionError: assert 100 == 2` (expected 2 repos, got 100)
102+
103+
**Root Cause**: Mock pagination logic returning repos indefinitely (hitting max_repos limit)
104+
105+
**Fix Strategy**:
106+
- Fix mock to properly simulate pagination exhaustion
107+
- Ensure `side_effect` returns empty list after N calls
108+
```python
109+
mock_paginated.side_effect = [
110+
[MockRepo("repo1"), MockRepo("repo2")], # First page
111+
[] # No more pages
112+
]
113+
```
114+
115+
### 7. Extract/Learn CLI Tests (18 failures)
116+
**Files**: `tests/unit/test_cli_extract_skills.py`, `tests/unit/test_cli_learn.py`
117+
118+
**Issues**:
119+
- `.skills-proposals` directory not created
120+
- No JSON files generated
121+
- Commands exiting with code 1
122+
123+
**Root Cause**: Commands expect valid assessment file in `.agentready/assessment-latest.json`
124+
125+
**Fix Strategy**:
126+
- Create proper assessment fixtures in test setup
127+
- Mock `LearningService` to avoid file I/O
128+
- Use Click's `isolated_filesystem()` for temporary directories
129+
130+
### 8. LLM Enricher Tests (2 failures)
131+
**Files**: `tests/unit/learners/test_llm_enricher.py`
132+
133+
**Issue**: `TypeError: APIStatusError.__init__() missing 2 required keyword-only arguments: 'response' and 'body'`
134+
135+
**Root Cause**: Anthropic SDK updated API error signatures
136+
137+
**Fix Strategy**:
138+
- Update mock errors to match new Anthropic SDK signatures
139+
```python
140+
from anthropic import APIStatusError, APIError
141+
# Update to include required params
142+
mock_error = APIStatusError(
143+
message="Rate limited",
144+
response=mock_response,
145+
body={}
146+
)
147+
```
148+
149+
### 9. Other Model/Reporter Tests (5 failures)
150+
**Files**: Various
151+
152+
**Issues**:
153+
- CSV reporter validation errors
154+
- Research formatter newline handling
155+
- Skill generator string formatting
156+
157+
**Fix Strategy**: Address individually based on specific failure
158+
159+
## Implementation Plan
160+
161+
### Phase 1: Quick Wins (Low-hanging fruit)
162+
1. Fix import path mocks (Align command tests)
163+
2. Fix Anthropic SDK error signatures (LLM enricher)
164+
3. Fix attribute.id vs attribute.attribute_id (Pattern extractor)
165+
4. Fix GitHub scanner pagination mocks
166+
167+
### Phase 2: Fixture Updates (Medium effort)
168+
1. Create reusable test fixture factories
169+
2. Add git initialization to repo fixtures
170+
3. Update assessment fixtures to include findings
171+
4. Add proper CLI test isolation
172+
173+
### Phase 3: Config Validation (Requires design decision)
174+
1. Decide: Model validation vs CLI validation
175+
2. Update Config model or test expectations
176+
3. Document validation strategy
177+
178+
## Testing Strategy
179+
180+
Run tests in isolation to verify fixes:
181+
```bash
182+
# Test specific categories
183+
pytest tests/unit/test_cli_align.py -v
184+
pytest tests/unit/learners/test_pattern_extractor.py -v
185+
pytest tests/unit/test_github_scanner.py -v
186+
187+
# Full suite
188+
pytest tests/ --no-cov -q
189+
```
190+
191+
## Success Criteria
192+
193+
- All 71 failing tests pass
194+
- No new test failures introduced
195+
- Coverage maintained or improved
196+
- Tests run in <2 minutes
197+
198+
## Files to Modify
199+
200+
1. `tests/unit/cli/test_main.py` - Config validation expectations
201+
2. `tests/unit/test_cli_align.py` - Import path mocks
202+
3. `tests/unit/learners/test_pattern_extractor.py` - Attribute field names
203+
4. `tests/unit/test_github_scanner.py` - Pagination mocks
204+
5. `tests/unit/test_learning_service.py` - Git repo fixtures
205+
6. `tests/unit/test_cli_extract_skills.py` - Assessment fixtures
206+
7. `tests/unit/test_cli_learn.py` - Assessment fixtures
207+
8. `tests/unit/learners/test_llm_enricher.py` - Anthropic SDK errors
208+
9. `tests/conftest.py` - Add shared fixtures (if needed)
209+
210+
## Notes
211+
212+
- These are **pre-existing failures**, not regressions from leaderboard PR
213+
- Leaderboard-specific tests (metadata, reporters) are all passing
214+
- Fix tests incrementally, commit after each category
215+
- Use conventional commits: `test: fix config validation test expectations`

‎src/agentready/cli/submit.py‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ def submit(repository, assessment_file, dry_run):
8686
click.echo(f"Error: Invalid assessment JSON (missing {e})", err=True)
8787
sys.exit(1)
8888

89-
# 4. Extract org/repo from URL
90-
if "github.com/" not in repo_url:
89+
# 4. Extract org/repo from URL (handle both HTTPS and SSH formats)
90+
if "github.com" not in repo_url:
9191
click.echo(
9292
"Error: Only GitHub repositories are supported for the leaderboard",
9393
err=True,
@@ -96,7 +96,13 @@ def submit(repository, assessment_file, dry_run):
9696
sys.exit(1)
9797

9898
try:
99-
org_repo = repo_url.split("github.com/")[1].strip("/").rstrip(".git")
99+
# Handle SSH format: git@github.com:org/repo.git
100+
if repo_url.startswith("git@github.com:"):
101+
org_repo = repo_url.split("git@github.com:")[1].rstrip(".git")
102+
# Handle HTTPS format: https://github.com/org/repo.git
103+
else:
104+
org_repo = repo_url.split("github.com/")[1].strip("/").rstrip(".git")
105+
100106
org, repo = org_repo.split("/")
101107
except (IndexError, ValueError):
102108
click.echo(f"Error: Could not parse GitHub repository from URL: {repo_url}")
@@ -157,8 +163,8 @@ def submit(repository, assessment_file, dry_run):
157163
click.echo(f"Error: Cannot access repository {org_repo}: {e}", err=True)
158164
sys.exit(1)
159165

160-
# 8. Fork agentready/agentready (if not already forked)
161-
upstream_repo = "agentready/agentready"
166+
# 8. Fork ambient-code/agentready (if not already forked)
167+
upstream_repo = "ambient-code/agentready"
162168
try:
163169
upstream = gh.get_repo(upstream_repo)
164170
click.echo(f"Found upstream: {upstream_repo}")

0 commit comments

Comments
 (0)