fix: Resolve 21 ruff linting errors (SIM102, SIM117, B904, SIM113, B007)
Fixed all 21 linting errors identified in GitHub Actions: SIM102 (7 errors - nested if statements): - config_extractor.py:468 - Combined nested conditions - config_validator.py (was B904, already fixed) - pattern_recognizer.py:430,538,916 - Combined nested conditions - test_example_extractor.py:365,412,460 - Combined nested conditions - unified_skill_builder.py:1070 - Combined nested conditions SIM117 (9 errors - multiple with statements): - test_install_agent.py:418 - Combined with statements - test_issue_219_e2e.py:278 - Combined with statements - test_llms_txt_downloader.py:33,88 - Combined with statements - test_skip_llms_txt.py:75,98,121,148,172,304 - Combined with statements B904 (1 error - exception handling): - config_validator.py:62 - Added 'from e' to exception chain SIM113 (1 error - enumerate usage): - doc_scraper.py:1068 - Removed unused 'completed' counter variable B007 (1 error - unused loop variable): - pdf_scraper.py:167 - Changed 'keywords' to '_' for unused variable All changes improve code quality without altering functionality. Tests: 1214 passed, 167 skipped (4 pre-existing failures unrelated) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
yusyus
@@ -415,9 +415,8 @@ class TestInstallAgentCLI:
|
||||
|
||||
def test_cli_requires_agent_flag(self):
|
||||
"""Test that CLI fails without --agent flag."""
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
with patch("sys.argv", ["install_agent.py", str(self.skill_dir)]):
|
||||
main()
|
||||
with pytest.raises(SystemExit) as exc_info, patch("sys.argv", ["install_agent.py", str(self.skill_dir)]):
|
||||
main()
|
||||
|
||||
# Missing required argument exits with code 2
|
||||
assert exc_info.value.code == 2
|
||||
|
||||
@@ -275,46 +275,45 @@ class TestIssue219Problem3CustomAPIEndpoints(unittest.TestCase):
|
||||
except ImportError:
|
||||
self.skipTest("anthropic package not installed")
|
||||
|
||||
with patch.dict(os.environ, {"ANTHROPIC_API_KEY": "test-key"}):
|
||||
with patch("skill_seekers.cli.enhance_skill.anthropic.Anthropic") as mock_anthropic:
|
||||
enhancer = SkillEnhancer(self.skill_dir)
|
||||
with patch.dict(os.environ, {"ANTHROPIC_API_KEY": "test-key"}), patch("skill_seekers.cli.enhance_skill.anthropic.Anthropic") as mock_anthropic:
|
||||
enhancer = SkillEnhancer(self.skill_dir)
|
||||
|
||||
# Mock response with ThinkingBlock (newer SDK)
|
||||
# ThinkingBlock has no .text attribute
|
||||
mock_thinking_block = SimpleNamespace(type="thinking")
|
||||
# Mock response with ThinkingBlock (newer SDK)
|
||||
# ThinkingBlock has no .text attribute
|
||||
mock_thinking_block = SimpleNamespace(type="thinking")
|
||||
|
||||
# TextBlock has .text attribute
|
||||
mock_text_block = SimpleNamespace(text="# Enhanced SKILL.md\n\nContent here")
|
||||
# TextBlock has .text attribute
|
||||
mock_text_block = SimpleNamespace(text="# Enhanced SKILL.md\n\nContent here")
|
||||
|
||||
mock_message = Mock()
|
||||
mock_message.content = [mock_thinking_block, mock_text_block]
|
||||
mock_message = Mock()
|
||||
mock_message.content = [mock_thinking_block, mock_text_block]
|
||||
|
||||
mock_client = mock_anthropic.return_value
|
||||
mock_client.messages.create.return_value = mock_message
|
||||
mock_client = mock_anthropic.return_value
|
||||
mock_client.messages.create.return_value = mock_message
|
||||
|
||||
# Read references (with proper metadata structure)
|
||||
references = {
|
||||
"index.md": {
|
||||
"content": "# Index\nTest content",
|
||||
"source": "documentation",
|
||||
"confidence": "high",
|
||||
"path": "index.md",
|
||||
"truncated": False,
|
||||
"size": 23,
|
||||
"repo_id": None,
|
||||
}
|
||||
# Read references (with proper metadata structure)
|
||||
references = {
|
||||
"index.md": {
|
||||
"content": "# Index\nTest content",
|
||||
"source": "documentation",
|
||||
"confidence": "high",
|
||||
"path": "index.md",
|
||||
"truncated": False,
|
||||
"size": 23,
|
||||
"repo_id": None,
|
||||
}
|
||||
}
|
||||
|
||||
# Call enhance_skill_md (should handle ThinkingBlock gracefully)
|
||||
result = enhancer.enhance_skill_md(references, current_skill_md="# Old")
|
||||
# Call enhance_skill_md (should handle ThinkingBlock gracefully)
|
||||
result = enhancer.enhance_skill_md(references, current_skill_md="# Old")
|
||||
|
||||
# VERIFY: Should find text from TextBlock, ignore ThinkingBlock
|
||||
self.assertIsNotNone(result, "Should return enhanced content")
|
||||
self.assertEqual(
|
||||
result,
|
||||
"# Enhanced SKILL.md\n\nContent here",
|
||||
"Should extract text from TextBlock",
|
||||
)
|
||||
# VERIFY: Should find text from TextBlock, ignore ThinkingBlock
|
||||
self.assertIsNotNone(result, "Should return enhanced content")
|
||||
self.assertEqual(
|
||||
result,
|
||||
"# Enhanced SKILL.md\n\nContent here",
|
||||
"Should extract text from TextBlock",
|
||||
)
|
||||
|
||||
|
||||
class TestIssue219IntegrationAll(unittest.TestCase):
|
||||
|
||||
@@ -30,9 +30,8 @@ def test_timeout_with_retry():
|
||||
"""Test timeout scenario with retry logic"""
|
||||
downloader = LlmsTxtDownloader("https://example.com/llms.txt", max_retries=2)
|
||||
|
||||
with patch("requests.get", side_effect=requests.Timeout("Connection timeout")) as mock_get:
|
||||
with patch("time.sleep") as mock_sleep: # Mock sleep to speed up test
|
||||
content = downloader.download()
|
||||
with patch("requests.get", side_effect=requests.Timeout("Connection timeout")) as mock_get, patch("time.sleep") as mock_sleep: # Mock sleep to speed up test
|
||||
content = downloader.download()
|
||||
|
||||
assert content is None
|
||||
assert mock_get.call_count == 2 # Should retry once (2 total attempts)
|
||||
@@ -85,9 +84,8 @@ def test_exponential_backoff():
|
||||
"""Test that exponential backoff delays are correct"""
|
||||
downloader = LlmsTxtDownloader("https://example.com/llms.txt", max_retries=3)
|
||||
|
||||
with patch("requests.get", side_effect=requests.Timeout("Connection timeout")):
|
||||
with patch("time.sleep") as mock_sleep:
|
||||
content = downloader.download()
|
||||
with patch("requests.get", side_effect=requests.Timeout("Connection timeout")), patch("time.sleep") as mock_sleep:
|
||||
content = downloader.download()
|
||||
|
||||
assert content is None
|
||||
# Should sleep with delays: 1s, 2s (2^0, 2^1)
|
||||
|
||||
@@ -72,11 +72,9 @@ class TestSkipLlmsTxtSyncBehavior(unittest.TestCase):
|
||||
os.chdir(tmpdir)
|
||||
converter = DocToSkillConverter(config, dry_run=False)
|
||||
|
||||
with patch.object(converter, "_try_llms_txt", return_value=False) as mock_try:
|
||||
with patch.object(converter, "scrape_page"):
|
||||
with patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
mock_try.assert_called_once()
|
||||
with patch.object(converter, "_try_llms_txt", return_value=False) as mock_try, patch.object(converter, "scrape_page"), patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
mock_try.assert_called_once()
|
||||
finally:
|
||||
os.chdir(original_cwd)
|
||||
|
||||
@@ -95,11 +93,9 @@ class TestSkipLlmsTxtSyncBehavior(unittest.TestCase):
|
||||
os.chdir(tmpdir)
|
||||
converter = DocToSkillConverter(config, dry_run=False)
|
||||
|
||||
with patch.object(converter, "_try_llms_txt") as mock_try:
|
||||
with patch.object(converter, "scrape_page"):
|
||||
with patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
mock_try.assert_not_called()
|
||||
with patch.object(converter, "_try_llms_txt") as mock_try, patch.object(converter, "scrape_page"), patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
mock_try.assert_not_called()
|
||||
finally:
|
||||
os.chdir(original_cwd)
|
||||
|
||||
@@ -118,10 +114,9 @@ class TestSkipLlmsTxtSyncBehavior(unittest.TestCase):
|
||||
os.chdir(tmpdir)
|
||||
converter = DocToSkillConverter(config, dry_run=True)
|
||||
|
||||
with patch.object(converter, "_try_llms_txt") as mock_try:
|
||||
with patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
mock_try.assert_not_called()
|
||||
with patch.object(converter, "_try_llms_txt") as mock_try, patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
mock_try.assert_not_called()
|
||||
finally:
|
||||
os.chdir(original_cwd)
|
||||
|
||||
@@ -145,11 +140,9 @@ class TestSkipLlmsTxtAsyncBehavior(unittest.TestCase):
|
||||
os.chdir(tmpdir)
|
||||
converter = DocToSkillConverter(config, dry_run=False)
|
||||
|
||||
with patch.object(converter, "_try_llms_txt", return_value=False) as mock_try:
|
||||
with patch.object(converter, "scrape_page_async", return_value=None):
|
||||
with patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
mock_try.assert_called_once()
|
||||
with patch.object(converter, "_try_llms_txt", return_value=False) as mock_try, patch.object(converter, "scrape_page_async", return_value=None), patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
mock_try.assert_called_once()
|
||||
finally:
|
||||
os.chdir(original_cwd)
|
||||
|
||||
@@ -169,11 +162,9 @@ class TestSkipLlmsTxtAsyncBehavior(unittest.TestCase):
|
||||
os.chdir(tmpdir)
|
||||
converter = DocToSkillConverter(config, dry_run=False)
|
||||
|
||||
with patch.object(converter, "_try_llms_txt") as mock_try:
|
||||
with patch.object(converter, "scrape_page_async", return_value=None):
|
||||
with patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
mock_try.assert_not_called()
|
||||
with patch.object(converter, "_try_llms_txt") as mock_try, patch.object(converter, "scrape_page_async", return_value=None), patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
mock_try.assert_not_called()
|
||||
finally:
|
||||
os.chdir(original_cwd)
|
||||
|
||||
@@ -301,11 +292,10 @@ class TestSkipLlmsTxtEdgeCases(unittest.TestCase):
|
||||
scrape_called.append(url)
|
||||
return None
|
||||
|
||||
with patch.object(converter, "scrape_page", side_effect=mock_scrape):
|
||||
with patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
# Should have attempted to scrape the base URL
|
||||
self.assertTrue(len(scrape_called) > 0)
|
||||
with patch.object(converter, "scrape_page", side_effect=mock_scrape), patch.object(converter, "save_summary"):
|
||||
converter.scrape_all()
|
||||
# Should have attempted to scrape the base URL
|
||||
self.assertTrue(len(scrape_called) > 0)
|
||||
finally:
|
||||
os.chdir(original_cwd)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user