-
Notifications
You must be signed in to change notification settings - Fork 0
test: autofix system validation with intentional failures #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,154 @@ | ||||||
| """Test file with intentional issues to validate autofix system. | ||||||
|
|
||||||
| This file contains: | ||||||
| 1. Black formatting violations | ||||||
| 2. Ruff lint errors | ||||||
|
Check failure on line 5 in tests/test_autofix_validation.py
|
||||||
| 3. Mypy type errors | ||||||
| 4. Failing tests | ||||||
| 5. Actual useful test coverage | ||||||
|
|
||||||
| Purpose: Validate the full autofix pipeline handles all failure modes. | ||||||
| """ | ||||||
|
|
||||||
| # --- BLACK VIOLATION: Bad formatting --- | ||||||
| import os,sys,time | ||||||
|
Check failure on line 14 in tests/test_autofix_validation.py
|
||||||
| from typing import Dict,List,Optional,Any | ||||||
|
Check failure on line 15 in tests/test_autofix_validation.py
|
||||||
|
||||||
| from typing import Dict,List,Optional,Any |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All async tests in the codebase use the @pytest.mark.asyncio decorator (see tests/test_adapter_base.py, tests/test_edgar.py, etc.). However, this file doesn't import pytest or use any pytest markers. If these tests are intended to be run as part of the test suite, they should follow the established conventions for test structure, including proper imports and decorators where applicable.
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'tracked_call' is not used.
| from adapters.base import connect_db,get_adapter,tracked_call | |
| from adapters.base import connect_db,get_adapter |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'json' is not used.
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 're' is not used.
| import re |
Check failure on line 22 in tests/test_autofix_validation.py
GitHub Actions / Python CI / lint-ruff
Ruff (I001)
tests/test_autofix_validation.py:14:1: I001 Import block is un-sorted or un-formatted
Check failure on line 22 in tests/test_autofix_validation.py
GitHub Actions / Python CI / lint-ruff
Ruff (I001)
tests/test_autofix_validation.py:14:1: I001 Import block is un-sorted or un-formatted
Check failure on line 22 in tests/test_autofix_validation.py
GitHub Actions / Python CI / lint-ruff
Ruff (I001)
tests/test_autofix_validation.py:14:1: I001 Import block is un-sorted or un-formatted
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'collections' is not used.
| import collections |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description lists E501 (line too long) as an intentional Ruff violation that should be caught, but the project's ruff configuration in pyproject.toml explicitly ignores E501 with ignore = ["E501"]. This intentional violation will not trigger a lint error and therefore will not validate the autofix system's handling of E501 errors.
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description lists Mypy type errors as intentional issues that should trigger Codex dispatch, but the project's mypy configuration in pyproject.toml excludes the tests directory and also has overrides that ignore errors in adapters. Since this test file imports from adapters.base, mypy type checking may not catch these errors as expected, preventing validation of the autofix system's handling of type errors.
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test manipulates environment variables directly with os.environ.pop(), but other tests in the codebase use pytest's monkeypatch fixture for environment variable management (see tests/test_adapter_base.py:16, tests/test_embeddings.py:31). Using monkeypatch ensures automatic cleanup and prevents test pollution. Consider accepting monkeypatch as a parameter and using monkeypatch.delenv() or monkeypatch.setenv() instead.
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic in this assertion is incorrect. The expression callable(getattr(adapter, "list_new_filings", None)) is False will only be True if the attribute exists and is explicitly the False object (not just falsy). This should use not callable(...) instead, or simply check hasattr(adapter, "list_new_filings"). The current logic makes this assertion always pass when the method exists and is callable, which defeats the purpose of the test.
| assert hasattr(adapter, "list_new_filings") or callable(getattr(adapter, "list_new_filings", None)) is False | |
| assert hasattr(adapter, "list_new_filings") and callable(getattr(adapter, "list_new_filings", None)) |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the existing test in tests/test_adapter_registry.py (line 10-13), adapters should be tested as modules with the pattern isinstance(adapter, types.ModuleType) and direct attribute checks using hasattr(). The current test uses callable(getattr(...)) which is inconsistent with the established testing pattern for adapters in this codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unconditional failing tests blocking suite
The three “intentional failure” tests here all fail unconditionally (41 == 42, followed by a KeyError and a TypeError in the two tests immediately below), and they are not marked xfail/skip. As written, any pytest run will halt on this block, keeping CI permanently red and preventing the rest of the suite from running; these should be guarded or deleted if the goal is a stable test run.
Useful? React with 👍 / 👎.
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test intentionally fails by asserting 41 == 42. While this is documented as an intentional failure for validation purposes, when this test runs it will cause the test suite to fail. Consider whether this test should be marked with a custom pytest marker (like @pytest.mark.skip(reason="Intentional failure for autofix validation")) until the autofix system is validated, to avoid breaking the main test suite.
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test intentionally raises a KeyError. While this is documented as an intentional failure, when executed it will cause the test suite to fail. The same concern applies as with the other intentionally failing tests - consider using pytest markers to skip these until the autofix system has been validated, to prevent disrupting the normal development workflow.
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable result is not used.
| result = data["nonexistent_key"] | |
| data["nonexistent_key"] |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test intentionally raises a TypeError. Like the other intentionally failing tests, this will cause test suite failures when executed. Consider using pytest skip markers for these validation tests to avoid disrupting normal CI/CD workflows until the autofix system has been properly validated.
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable result is not used.
| result = value + 5 | |
| value + 5 |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing for None should use the 'is' operator.
| if value == None: | |
| if value is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'sys' is not used.
Import of 'time' is not used.