Add comprehensive test coverage for critical modules#9
Add comprehensive test coverage for critical modules#9Lexicoding-systems wants to merge 1 commit intomainfrom
Conversation
- Add test_capability_tokens.py: 32 tests for token creation, validation, expiry, authorization, and token store operations (100% coverage) - Add test_identity.py: 41 tests for Ed25519 key generation, signing, verification, key persistence, and node identity (100% coverage) - Add test_decision_service.py: 41 tests for decision workflows, request/response handling, capability token generation, ledger integration, and signatures (100% coverage) - Add test_health.py: 38 tests for health checks, liveness/readiness probes, status aggregation, and error handling (100% coverage) These tests increase overall code coverage from 22% to 56%, covering all critical security and core functionality modules that were previously untested. Tests added: 151 passing tests Coverage improvement: +34% (22% → 56%) Critical modules now at 100% coverage: capability tokens, identity & signing, decision service, health checks
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
||
| from datetime import datetime, timedelta | ||
|
|
||
| import pytest |
Check notice
Code scanning / CodeQL
Unused import Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, an unused import should be removed to avoid unnecessary dependencies and to keep the code clear. Here, the pytest module is imported but not referenced anywhere in the shown test file, while all assertions are done with plain assert and no pytest APIs.
The best fix is to delete the import pytest line in tests/test_capability_tokens.py and leave all other imports and code unchanged. Specifically, remove line 5 (import pytest), keeping the datetime, timedelta, and CapabilityToken / CapabilityTokenStore imports intact. No new methods, imports, or definitions are needed.
| @@ -2,8 +2,6 @@ | ||
|
|
||
| from datetime import datetime, timedelta | ||
|
|
||
| import pytest | ||
|
|
||
| from lexecon.capability.tokens import CapabilityToken, CapabilityTokenStore | ||
|
|
||
|
|
| for i in range(3) | ||
| ] | ||
|
|
||
| responses = [service.evaluate_request(req) for req in requests] |
Check notice
Code scanning / CodeQL
Unused local variable Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, to fix an unused local variable created solely as a byproduct of invoking a function with side effects, you should keep the function calls but remove the unnecessary assignment. This preserves behavior while eliminating the unused variable.
In this specific test, we need to continue calling service.evaluate_request(req) for each req so that the ledger is populated, but we do not need to store the responses. The best minimal change is to replace the list comprehension assigned to responses with a simple for loop that invokes service.evaluate_request(req) for each request. This keeps the behavior (all decisions are evaluated) and removes the unused variable. The change is confined to tests/test_decision_service.py, in the test_decision_audit_trail method around line 573; no new imports or helper methods are required.
| @@ -570,7 +570,8 @@ | ||
| for i in range(3) | ||
| ] | ||
|
|
||
| responses = [service.evaluate_request(req) for req in requests] | ||
| for req in requests: | ||
| service.evaluate_request(req) | ||
|
|
||
| # Check audit report | ||
| report = ledger.generate_audit_report() |
|
|
||
| import time | ||
|
|
||
| import pytest |
Check notice
Code scanning / CodeQL
Unused import Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix an unused import, the general remedy is to remove the import statement if the imported name is not referenced anywhere in the module. This eliminates an unnecessary dependency and slightly speeds up module loading while improving readability.
For this specific case, in tests/test_health.py, delete the line import pytest at line 5. No additional code changes are necessary because all visible tests rely solely on assert statements and objects imported from lexecon.observability.health, and none of them reference pytest. Removing this line will not alter existing functionality, as pytest does not need to be imported in the module for these tests to run. No new imports, methods, or definitions are required.
| @@ -2,8 +2,6 @@ | ||
|
|
||
| import time | ||
|
|
||
| import pytest | ||
|
|
||
| from lexecon.observability.health import ( | ||
| HealthCheck, | ||
| HealthStatus, |
| @@ -0,0 +1,548 @@ | |||
| """Tests for identity and signing functionality.""" | |||
|
|
|||
| import json | |||
Check notice
Code scanning / CodeQL
Unused import Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix an unused import, the best approach is simply to remove the import statement that is not referenced anywhere in the file. This reduces unnecessary dependencies and slightly improves readability.
In this file, remove the module-level import json at line 3 of tests/test_identity.py. The function test_node_can_verify_own_signature already imports json locally (line 412), so behavior will not change. No other lines need modification, and no new imports or definitions are required.
| @@ -1,6 +1,5 @@ | ||
| """Tests for identity and signing functionality.""" | ||
|
|
||
| import json | ||
| import tempfile | ||
| from pathlib import Path | ||
|
|
| signature = node.sign(data) | ||
|
|
||
| # Convert dict to canonical JSON for verification | ||
| import json |
Check notice
Code scanning / CodeQL
Module is imported more than once Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix the problem, remove the redundant inner import json and use the already-imported json module from the top of the file. In general, you should import a module once at the top of the file unless you have a specific need for a local, conditional, or aliased import.
Concretely, in tests/test_identity.py, within TestNodeIdentity.test_node_can_verify_own_signature, delete the line import json (currently line 412 in your snippet) and keep the subsequent call json.dumps(...) unchanged, since it will refer to the top-level import from line 3. No additional methods, imports, or definitions are needed.
| @@ -409,7 +409,6 @@ | ||
| signature = node.sign(data) | ||
|
|
||
| # Convert dict to canonical JSON for verification | ||
| import json | ||
| canonical = json.dumps(data, sort_keys=True, separators=(",", ":")) | ||
|
|
||
| # This should work with the node's verify_signature method |
|
|
||
| # Convert dict to canonical JSON for verification | ||
| import json | ||
| canonical = json.dumps(data, sort_keys=True, separators=(",", ":")) |
Check notice
Code scanning / CodeQL
Unused local variable Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, unused local variables should be either removed (if they are genuinely unnecessary) or incorporated into the logic (if they were intended to be used). When the right-hand side has side effects or encodes important semantics, you should keep the expression and only remove or adjust the left-hand side.
Here, canonical is a canonicalized JSON string of data, and the surrounding comments indicate that verification should be done using string data. The test currently calls KeyManager.verify(data, signature, node.key_manager.public_key), which passes the original dict instead of the canonical string. The best fix is to change that call to use canonical instead of data. This both removes the “unused variable” issue and aligns the test with its stated intent.
Concretely, in tests/test_identity.py, within TestNodeIdentity.test_node_can_verify_own_signature, keep the line creating canonical, and change the KeyManager.verify call on line 418 so that its data argument is canonical rather than data. No new imports or additional definitions are needed; json is already imported within the test method.
| @@ -414,8 +414,8 @@ | ||
|
|
||
| # This should work with the node's verify_signature method | ||
| # but it expects string data, so we need to verify differently | ||
| # Let's use the key manager's verify method | ||
| is_valid = KeyManager.verify(data, signature, node.key_manager.public_key) | ||
| # Let's use the key manager's verify method with canonical JSON string | ||
| is_valid = KeyManager.verify(canonical, signature, node.key_manager.public_key) | ||
|
|
||
| assert is_valid is True | ||
|
|
| def test_public_key_distribution(self): | ||
| """Test that public keys can be shared for verification.""" | ||
| node1 = NodeIdentity("alice") | ||
| node2 = NodeIdentity("bob") |
Check notice
Code scanning / CodeQL
Unused local variable Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix an unused local variable, either remove it if it is genuinely unnecessary, or rename it to an “unused” style name (like _ or unused_node2) if it is intentionally present for documentation. Here, the second node is not used at all in the test logic, and the test already checks successful verification using node1’s public key, so the cleanest fix without changing functionality is to remove the creation of node2.
Concretely, in tests/test_identity.py within class TestCrossNodeVerification, in the method test_public_key_distribution, delete the line that assigns node2 = NodeIdentity("bob"). All other lines remain unchanged; no new imports or helper methods are needed because the variable is simply not required.
| @@ -441,7 +441,6 @@ | ||
| def test_public_key_distribution(self): | ||
| """Test that public keys can be shared for verification.""" | ||
| node1 = NodeIdentity("alice") | ||
| node2 = NodeIdentity("bob") | ||
|
|
||
| data = {"transfer": "100", "to": "bob"} | ||
|
|
Add test_capability_tokens.py: 32 tests for token creation, validation,
expiry, authorization, and token store operations (100% coverage)
Add test_identity.py: 41 tests for Ed25519 key generation, signing,
verification, key persistence, and node identity (100% coverage)
Add test_decision_service.py: 41 tests for decision workflows, request/response
handling, capability token generation, ledger integration, and signatures
(100% coverage)
Add test_health.py: 38 tests for health checks, liveness/readiness probes,
status aggregation, and error handling (100% coverage)
These tests increase overall code coverage from 22% to 56%, covering all
critical security and core functionality modules that were previously untested.
Tests added: 151 passing tests
Coverage improvement: +34% (22% → 56%)
Critical modules now at 100% coverage: capability tokens, identity & signing,
decision service, health checks