fix(acms): resolve CI failures in budget enforcement PR #9673
- Fix BDD test state leakage by unconditionally resetting BudgetEnforcer
in step_create_budget_enforcer_with_max_total_size instead of using
conditional if/else reconstruction that skipped the first Background
step's max_file_size from being preserved.
- Add explicit type annotations (context: object) to all Behave step
function signatures for Pyright compliance.
- Fix ruff format compliance by standardizing decorator string
formatting in acms_budget_enforcement_steps.py.
- Correct __init__.py exports: renamed _uks_exports -> _uko_exports
typo and fixed the __all__ reference to use the corrected variable.
- Add Robot Framework integration test helper using inline imports
(from cleveragents.acms.budget_enforcement import BudgetEnforcer)
instead of sys.path manipulation, and update robot tests to use
python3 and ${WORKSPACE} paths for CI compatibility.
ISSUES CLOSED: #9583
This commit is contained in:
@@ -1,8 +1,6 @@
|
||||
*** Settings ***
|
||||
Documentation Integration tests for ACMS BudgetEnforcer max_file_size and max_total_size constraints
|
||||
Resource ${CURDIR}/common.resource
|
||||
Suite Setup Setup Test Environment
|
||||
Suite Teardown Cleanup Test Environment
|
||||
|
||||
*** Variables ***
|
||||
${HELPER_SCRIPT} robot/helper_acms_budget_enforcement.py
|
||||
@@ -10,60 +8,60 @@ ${HELPER_SCRIPT} robot/helper_acms_budget_enforcement.py
|
||||
*** Test Cases ***
|
||||
Budget Enforcer Includes File Within Max File Size
|
||||
[Documentation] BudgetEnforcer includes a file within max_file_size limit
|
||||
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} file-within-limit cwd=${WORKSPACE}
|
||||
${result}= Run Process python3 ${WORKSPACE}/${HELPER_SCRIPT} file-within-limit cwd=${WORKSPACE}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} file-within-limit-ok
|
||||
|
||||
Budget Enforcer Excludes File Exceeding Max File Size
|
||||
[Documentation] BudgetEnforcer excludes a file that exceeds max_file_size
|
||||
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} file-exceeds-limit cwd=${WORKSPACE}
|
||||
${result}= Run Process python3 ${WORKSPACE}/${HELPER_SCRIPT} file-exceeds-limit cwd=${WORKSPACE}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} file-exceeds-limit-ok
|
||||
|
||||
Budget Enforcer Enforces Max Total Size
|
||||
[Documentation] BudgetEnforcer cuts off files when max_total_size is reached
|
||||
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} total-size-cutoff cwd=${WORKSPACE}
|
||||
${result}= Run Process python3 ${WORKSPACE}/${HELPER_SCRIPT} total-size-cutoff cwd=${WORKSPACE}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} total-size-cutoff-ok
|
||||
|
||||
Budget Enforcer Violation Message Is Clear
|
||||
[Documentation] BudgetViolation message clearly describes the constraint exceeded
|
||||
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} violation-message cwd=${WORKSPACE}
|
||||
${result}= Run Process python3 ${WORKSPACE}/${HELPER_SCRIPT} violation-message cwd=${WORKSPACE}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} violation-message-ok
|
||||
|
||||
Budget Enforcer Returns Defensive Copies
|
||||
[Documentation] get_violations and get_included_files return defensive copies
|
||||
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} defensive-copies cwd=${WORKSPACE}
|
||||
${result}= Run Process python3 ${WORKSPACE}/${HELPER_SCRIPT} defensive-copies cwd=${WORKSPACE}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} defensive-copies-ok
|
||||
|
||||
Budget Enforcer Validates Constructor Arguments
|
||||
[Documentation] BudgetEnforcer raises ValueError for invalid budget values
|
||||
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} constructor-validation cwd=${WORKSPACE}
|
||||
${result}= Run Process python3 ${WORKSPACE}/${HELPER_SCRIPT} constructor-validation cwd=${WORKSPACE}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} constructor-validation-ok
|
||||
|
||||
Budget Enforcer Validates Add File Arguments
|
||||
[Documentation] add_file raises ValueError for empty name
|
||||
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} add-file-validation cwd=${WORKSPACE}
|
||||
${result}= Run Process python3 ${WORKSPACE}/${HELPER_SCRIPT} add-file-validation cwd=${WORKSPACE}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} add-file-validation-ok
|
||||
|
||||
Budget Enforcer Handles Multi-Byte UTF-8 Content
|
||||
[Documentation] BudgetEnforcer measures file size in bytes not characters
|
||||
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} multibyte-utf8 cwd=${WORKSPACE}
|
||||
${result}= Run Process python3 ${WORKSPACE}/${HELPER_SCRIPT} multibyte-utf8 cwd=${WORKSPACE}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} multibyte-utf8-ok
|
||||
|
||||
Budget Enforcer Reset Clears State
|
||||
[Documentation] reset() clears all included files, violations, and total size
|
||||
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} reset-state cwd=${WORKSPACE}
|
||||
${result}= Run Process python3 ${WORKSPACE}/${HELPER_SCRIPT} reset-state cwd=${WORKSPACE}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} reset-state-ok
|
||||
|
||||
Budget Enforcer Assembled Context Joins Files
|
||||
[Documentation] get_assembled_context joins included file contents with newlines
|
||||
${result}= Run Process ${PYTHON} ${HELPER_SCRIPT} assembled-context cwd=${WORKSPACE}
|
||||
${result}= Run Process python3 ${WORKSPACE}/${HELPER_SCRIPT} assembled-context cwd=${WORKSPACE}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} assembled-context-ok
|
||||
|
||||
@@ -5,13 +5,11 @@ from __future__ import annotations
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src"))
|
||||
|
||||
from cleveragents.acms.budget_enforcement import BudgetEnforcer
|
||||
|
||||
|
||||
def _test_file_within_limit() -> None:
|
||||
"""BudgetEnforcer includes a file within max_file_size limit."""
|
||||
from cleveragents.acms.budget_enforcement import BudgetEnforcer
|
||||
|
||||
enforcer = BudgetEnforcer(max_file_size=1000, max_total_size=5000)
|
||||
content = "x" * 500
|
||||
included = enforcer.add_file("small.txt", content)
|
||||
@@ -30,6 +28,8 @@ def _test_file_within_limit() -> None:
|
||||
|
||||
def _test_file_exceeds_limit() -> None:
|
||||
"""BudgetEnforcer excludes a file that exceeds max_file_size."""
|
||||
from cleveragents.acms.budget_enforcement import BudgetEnforcer
|
||||
|
||||
enforcer = BudgetEnforcer(max_file_size=1000, max_total_size=5000)
|
||||
content = "x" * 1500
|
||||
included = enforcer.add_file("large.txt", content)
|
||||
@@ -53,8 +53,9 @@ def _test_file_exceeds_limit() -> None:
|
||||
|
||||
def _test_total_size_cutoff() -> None:
|
||||
"""BudgetEnforcer cuts off files when max_total_size is reached."""
|
||||
from cleveragents.acms.budget_enforcement import BudgetEnforcer
|
||||
|
||||
enforcer = BudgetEnforcer(max_file_size=1000, max_total_size=2500)
|
||||
# Add 3 files of 1000 bytes each; third should be cut off (total would be 3000)
|
||||
assert enforcer.add_file("f1.txt", "x" * 1000) is True
|
||||
assert enforcer.add_file("f2.txt", "x" * 1000) is True
|
||||
assert enforcer.add_file("f3.txt", "x" * 1000) is False
|
||||
@@ -62,7 +63,9 @@ def _test_total_size_cutoff() -> None:
|
||||
f"Expected total size 2000, got {enforcer.get_total_size()}"
|
||||
)
|
||||
violations = enforcer.get_violations()
|
||||
total_violations = [v for v in violations if v.violation_type == "max_total_size"]
|
||||
total_violations = [
|
||||
v for v in violations if v.violation_type == "max_total_size"
|
||||
]
|
||||
assert len(total_violations) == 1, (
|
||||
f"Expected 1 max_total_size violation, got {len(total_violations)}"
|
||||
)
|
||||
@@ -71,6 +74,8 @@ def _test_total_size_cutoff() -> None:
|
||||
|
||||
def _test_violation_message() -> None:
|
||||
"""BudgetViolation message clearly describes the constraint exceeded."""
|
||||
from cleveragents.acms.budget_enforcement import BudgetEnforcer
|
||||
|
||||
enforcer = BudgetEnforcer(max_file_size=500, max_total_size=2000)
|
||||
enforcer.add_file("big.txt", "x" * 600)
|
||||
violations = enforcer.get_violations()
|
||||
@@ -85,11 +90,12 @@ def _test_violation_message() -> None:
|
||||
|
||||
def _test_defensive_copies() -> None:
|
||||
"""get_violations and get_included_files return defensive copies."""
|
||||
from cleveragents.acms.budget_enforcement import BudgetEnforcer
|
||||
|
||||
enforcer = BudgetEnforcer(max_file_size=1000, max_total_size=5000)
|
||||
enforcer.add_file("f1.txt", "x" * 500)
|
||||
enforcer.add_file("too_big.txt", "x" * 1500)
|
||||
|
||||
# Mutate the returned lists — should not affect enforcer state
|
||||
violations = enforcer.get_violations()
|
||||
violations.clear()
|
||||
assert len(enforcer.get_violations()) == 1, (
|
||||
@@ -106,28 +112,30 @@ def _test_defensive_copies() -> None:
|
||||
|
||||
def _test_constructor_validation() -> None:
|
||||
"""BudgetEnforcer raises ValueError for invalid budget values."""
|
||||
# Negative max_file_size
|
||||
from cleveragents.acms.budget_enforcement import BudgetEnforcer
|
||||
|
||||
try:
|
||||
BudgetEnforcer(max_file_size=-1, max_total_size=5000)
|
||||
raise AssertionError("Expected ValueError for negative max_file_size")
|
||||
except ValueError as e:
|
||||
assert "max_file_size" in str(e), f"Error should mention max_file_size: {e}"
|
||||
assert "max_file_size" in str(e), (
|
||||
f"Error should mention max_file_size: {e}"
|
||||
)
|
||||
|
||||
# Zero max_total_size
|
||||
try:
|
||||
BudgetEnforcer(max_file_size=100, max_total_size=0)
|
||||
raise AssertionError("Expected ValueError for zero max_total_size")
|
||||
except ValueError as e:
|
||||
assert "max_total_size" in str(e), f"Error should mention max_total_size: {e}"
|
||||
assert "max_total_size" in str(e), (
|
||||
f"Error should mention max_total_size: {e}"
|
||||
)
|
||||
|
||||
# max_file_size > max_total_size
|
||||
try:
|
||||
BudgetEnforcer(max_file_size=5000, max_total_size=1000)
|
||||
raise AssertionError("Expected ValueError for max_file_size > max_total_size")
|
||||
except ValueError as e:
|
||||
assert "cannot exceed" in str(e), f"Error should mention cannot exceed: {e}"
|
||||
|
||||
# Valid construction should succeed
|
||||
enforcer = BudgetEnforcer(max_file_size=1000, max_total_size=5000)
|
||||
assert enforcer.max_file_size == 1000
|
||||
assert enforcer.max_total_size == 5000
|
||||
@@ -136,16 +144,16 @@ def _test_constructor_validation() -> None:
|
||||
|
||||
def _test_add_file_validation() -> None:
|
||||
"""add_file raises ValueError for empty name."""
|
||||
from cleveragents.acms.budget_enforcement import BudgetEnforcer
|
||||
|
||||
enforcer = BudgetEnforcer(max_file_size=1000, max_total_size=5000)
|
||||
|
||||
# Empty name
|
||||
try:
|
||||
enforcer.add_file("", "some content")
|
||||
raise AssertionError("Expected ValueError for empty name")
|
||||
except ValueError as e:
|
||||
assert "name" in str(e).lower(), f"Error should mention name: {e}"
|
||||
|
||||
# Non-string content
|
||||
try:
|
||||
enforcer.add_file("file.txt", object())
|
||||
raise AssertionError("Expected TypeError for non-string content")
|
||||
@@ -157,13 +165,12 @@ def _test_add_file_validation() -> None:
|
||||
|
||||
def _test_multibyte_utf8() -> None:
|
||||
"""BudgetEnforcer measures file size in bytes not characters."""
|
||||
from cleveragents.acms.budget_enforcement import BudgetEnforcer
|
||||
|
||||
enforcer = BudgetEnforcer(max_file_size=1000, max_total_size=5000)
|
||||
|
||||
# Emoji character U+1F600 is 4 bytes in UTF-8
|
||||
# 251 emoji * 4 bytes = 1004 bytes > 1000 byte limit
|
||||
emoji_char = "\U0001f600"
|
||||
content = emoji_char * 251 # 251 chars but 1004 bytes
|
||||
assert len(content) == 251, "Character count should be 251"
|
||||
emoji_char = "\U0001f600" # U+1F600 -- 4 bytes in UTF-8
|
||||
content = emoji_char * 251
|
||||
assert len(content.encode("utf-8")) == 1004, "Byte count should be 1004"
|
||||
|
||||
included = enforcer.add_file("emoji.txt", content)
|
||||
@@ -177,20 +184,20 @@ def _test_multibyte_utf8() -> None:
|
||||
f"Violation should report byte size 1004, got {violations[0].file_size}"
|
||||
)
|
||||
|
||||
# A file with 250 emoji (1000 bytes) should be at the boundary — included
|
||||
# (strict greater-than: 1000 > 1000 is False)
|
||||
enforcer2 = BudgetEnforcer(max_file_size=1000, max_total_size=5000)
|
||||
content_boundary = emoji_char * 250 # 250 chars, 1000 bytes
|
||||
content_boundary = emoji_char * 250
|
||||
assert len(content_boundary.encode("utf-8")) == 1000
|
||||
included2 = enforcer2.add_file("boundary.txt", content_boundary)
|
||||
assert included2 is True, (
|
||||
"File with exactly 1000 bytes should be included (boundary is inclusive)"
|
||||
"File with exactly 1000 bytes should be included"
|
||||
)
|
||||
print("multibyte-utf8-ok")
|
||||
|
||||
|
||||
def _test_reset_state() -> None:
|
||||
"""reset() clears all included files, violations, and total size."""
|
||||
from cleveragents.acms.budget_enforcement import BudgetEnforcer
|
||||
|
||||
enforcer = BudgetEnforcer(max_file_size=1000, max_total_size=5000)
|
||||
enforcer.add_file("f1.txt", "x" * 500)
|
||||
enforcer.add_file("too_big.txt", "x" * 1500)
|
||||
@@ -211,7 +218,6 @@ def _test_reset_state() -> None:
|
||||
"After reset, violations should be empty"
|
||||
)
|
||||
|
||||
# Should be able to add files again after reset
|
||||
assert enforcer.add_file("new.txt", "y" * 300) is True
|
||||
assert enforcer.get_total_size() == 300
|
||||
print("reset-state-ok")
|
||||
@@ -219,6 +225,8 @@ def _test_reset_state() -> None:
|
||||
|
||||
def _test_assembled_context() -> None:
|
||||
"""get_assembled_context joins included file contents with newlines."""
|
||||
from cleveragents.acms.budget_enforcement import BudgetEnforcer
|
||||
|
||||
enforcer = BudgetEnforcer(max_file_size=1000, max_total_size=5000)
|
||||
enforcer.add_file("a.txt", "hello")
|
||||
enforcer.add_file("b.txt", "world")
|
||||
@@ -226,7 +234,6 @@ def _test_assembled_context() -> None:
|
||||
context = enforcer.get_assembled_context()
|
||||
assert context == "hello\nworld", f"Expected 'hello\\nworld', got {context!r}"
|
||||
|
||||
# Empty enforcer returns empty string
|
||||
enforcer2 = BudgetEnforcer(max_file_size=1000, max_total_size=5000)
|
||||
assert enforcer2.get_assembled_context() == "", (
|
||||
"Empty enforcer should return empty string"
|
||||
@@ -234,7 +241,7 @@ def _test_assembled_context() -> None:
|
||||
print("assembled-context-ok")
|
||||
|
||||
|
||||
_TESTS = {
|
||||
_TESTS: dict[str, callable] = {
|
||||
"file-within-limit": _test_file_within_limit,
|
||||
"file-exceeds-limit": _test_file_exceeds_limit,
|
||||
"total-size-cutoff": _test_total_size_cutoff,
|
||||
@@ -248,7 +255,7 @@ _TESTS = {
|
||||
}
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
if __name__ == "__main__": # noqa: C901
|
||||
if len(sys.argv) < 2 or sys.argv[1] not in _TESTS:
|
||||
print(
|
||||
f"Usage: {sys.argv[0]} <{'|'.join(sorted(_TESTS))}>",
|
||||
|
||||
Reference in New Issue
Block a user