forked from cleveragents/cleveragents-core
cbf8bcc993
## Summary Add `robot/e2e/m5_acceptance.robot` with **21 zero-mock E2E test cases** (in addition to existing M5 test suite) covering all M5 (v3.4.0) acceptance criteria: 1. **Context Assembly** — add/list/show/clear files in the context pipeline 2. **Context Scaling** — 10,000+ file project setup with simulate plumbing *(structural)* 3. **Context Policy Configuration** — per-view include/exclude paths, file-size limits 4. **Budget Enforcement** — max_file_size / max_total_size constraint storage *(structural)* 5. **Context Analysis** — ACMS pipeline inspect (tier schema) and simulate (JSON schema) *(structural)* 6. **Plan Execution** — real LLM calls via `openai/gpt-4o-mini` (`plan use` + `plan resume`) ### Structural vs. Behavioural Scope Tests in sections 1b–4 that use `project context simulate` or `inspect` are **structural / plumbing validations** — they verify CLI execution, JSON serialization, and stored configuration but do **not** exercise actual ACMS indexing or budget enforcement because the `ContextTierService` is an in-memory singleton that starts empty per CLI process. Each affected test has a `[Documentation]` note explaining this limitation. Behavioural ACMS validation is deferred until the full indexing pipeline is wired. ### Production Bug Fixes | Fix | File | Description | |-----|------|-------------| | `session.flush()` → `session.commit()` | `project_context.py` | Policy changes silently lost on `session.close()` | | `contextlib.suppress` rollback wrapper | `project_context.py` | Prevents rollback failure from masking original commit exception | | Add `session_factory` DI provider | `container.py` | `project context` commands hit `AttributeError` | | `providers.Factory` → `providers.Singleton` | `container.py` | Avoid creating duplicate engines per call | | Add Gemini API key pattern | `redaction.py` | `AIzaSy...` keys now redacted in logs | ### Review Feedback Addressed (Tenth Pass — @CoreRasurae Review #2410) | # | Severity | Finding | Fix | |---|----------|---------|-----| | P3-1 | Medium | "Clear Context" test tautological — never asserts files were present before clearing | Added `Should Contain ${list_before.stdout} config.py` precondition check after `context-load` and before `clear` | | P3-2 | Medium | Policy/budget verification uses substring matching (`Should Contain 262144`) | Replaced with `Extract JSON From Stdout` + `$rv.get('max_file_size') == 262144` parsed JSON assertions using `resolved_view` dict access | | P3-3 | Medium | Plan resume doesn't verify `phase` value, only existence | Added `Should Not Be Equal As Strings ${phase} queued` assertion to verify plan transitioned from queued | | P3-4 | Medium | Plan JSON extraction inconsistency (`rindex` vs `Extract JSON From Stdout`) | Replaced fragile `rindex`-based extraction with `Extract JSON From Stdout` keyword for consistency | | P3-6 | Medium | Context show summary weak content assertions | Added `Should Not Contain` guards against traceback/error output to reject false positives | | P3-8 | Medium | `_SafeSession` singleton may accumulate dirty state after rollback | Changed `_SafeSession.close()` from pure no-op to `real.rollback()` to reset session state between calls | | P3-14 | Medium | No test for `_save_policy_json` rollback path | Added BDD scenario "Save policy rollback re-raises after commit failure" with monkey-patched commit | | P3-15 | Medium | No test for `_save_policy_json` on nonexistent project | Added BDD scenario "Save policy on nonexistent project row updates zero rows" verifying silent 0-row behavior | | P4-1 | Low | Plan resume TRY/EXCEPT swallows assertion details | Moved field assertions outside TRY block; TRY only guards JSON extraction | | P4-2 | Low | `Safe Parse Json Field` logs stale error context | Fixed to track and report both Strategy 1 and Strategy 2 error contexts separately | | P4-4 | Low | SQLite WAL/SHM files not cleaned in regression test | Added cleanup loop for `-wal` and `-shm` suffixes alongside `.db` file | ### Deferred Items (Out of Scope) | ID | Severity | Reason | |----|----------|--------| | P2-1 | High | `execution_environment` silently dropped on subsequent `context set` — pre-existing production code bug in `_write_policy()`, not introduced by this PR | | P2-2 | High | Unhandled `ValidationError` on corrupt policy blob — pre-existing `_read_policy()` code, not changed by this PR | | P2-3 | High | Silent no-op UPDATE when `ns_projects` row missing — pre-existing `_save_policy_json` logic; this PR only changed error handling | | P3-5 | Medium | Structural tests cannot detect regressions — already honestly documented in every affected test's `[Documentation]` block | | P3-7 | Medium | View inheritance/override behavior not tested — nice-to-have, not in ticket acceptance criteria | | P3-9 | Medium | `context_set` double-writes when `execution_environment` set — pre-existing production logic | | P3-10 | Medium | `budget_tokens=0` silently replaced by default (falsy `or`) — pre-existing production code bug | | P3-11 | Medium | `context set` replaces entire view instead of merging — pre-existing design choice | | P3-12 | Medium | GEMINI_API_KEY propagated but potentially unused — security-first: propagating for redaction testing | | P3-13 | Medium | `reset_container()` doesn't dispose Singleton resources — pre-existing container lifecycle issue | | M5 | Medium | `_build_session_factory` engine never disposed — production code architecture, out of scope for testing ticket | | M6 | Medium | Missing `check_same_thread`/`isolation_level` — production code architecture, out of scope for testing ticket | | L1 | Low | `plan resume` not in spec CLI synopsis — informational | | L2 | Low | Context summary assertions depend on exact CLI wording — acceptable stability risk | | L3 | Low | Gemini regex minimum length slightly loose — acceptable security-first trade-off | | L4 | Low | Missing Google OAuth2 credential patterns — out of scope for this PR | | P4-3 | Low | `Run CLI` keyword duplicated — different purpose (uses `${WS}` as default cwd), not a true duplicate | | P4-5–P4-9 | Low | Various additional E2E coverage gaps — nice-to-have, not in ticket acceptance criteria | ### Quality Gates | Gate | Result | |------|--------| | lint | PASS | | typecheck | PASS (0 errors) | | unit_tests | **393/393** features, 11,210 scenarios | | integration_tests | **1,576/1,576** | | e2e_tests | **37/37** (21 M5 + 12 M6 + 2 smoke + 2 M1) | | coverage_report | **97%** (threshold: 97%) | ### Files Changed | File | Change | |------|--------| | `robot/e2e/m5_acceptance.robot` | **NEW** — 21 E2E test cases with honest structural documentation, parsed JSON assertions, prerequisite skip guards on all sections, safe assertion messages | | `robot/e2e/common_e2e.resource` | `on_timeout=kill` + return code checks + safe key evaluation via `os.environ.get` + fixed stale error logging in `Safe Parse Json Field` | | `robot/e2e/m1_acceptance.robot` | `on_timeout=kill` on git log | | `robot/e2e/m2_acceptance.robot` | `on_timeout=kill` + return code checks + safe assertion messages (no stderr embedding) | | `src/cleveragents/application/container.py` | Add `_build_session_factory` + `session_factory` Singleton | | `src/cleveragents/cli/commands/project_context.py` | `flush()` → `commit()` + `contextlib.suppress` rollback | | `src/cleveragents/shared/redaction.py` | Add Gemini API key pattern | | `noxfile.py` | Propagate `GEMINI_API_KEY` in e2e_tests | | `CHANGELOG.md` | 4 entries for #745 | | `features/application_container_coverage_boost.feature` | Updated title + 3 scenarios | | `features/steps/application_container_coverage_boost_steps.py` | Step defs for `_build_session_factory` | | `features/consolidated_security.feature` | 2 Gemini API key redaction scenarios | | `features/project_context_cli_coverage_boost.feature` | `flush()→commit()` regression test + rollback path + nonexistent project tests | | `features/steps/project_context_cli_coverage_boost_steps.py` | Separate engines for regression test + `try/finally` cleanup + `_SafeSession.close()` state reset + rollback/nonexistent test steps + WAL/SHM cleanup | Closes #745 ISSUES CLOSED: #745 Reviewed-on: cleveragents/cleveragents-core#811 Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com> Co-authored-by: Rui Hu <rui.hu@cleverthis.com> Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
160 lines
8.2 KiB
Plaintext
160 lines
8.2 KiB
Plaintext
*** Settings ***
|
|
Documentation Common resources and keywords for E2E Robot Framework tests.
|
|
...
|
|
... E2E tests use **zero mocking** — they exercise the real
|
|
... CleverAgents CLI with real LLM API keys. This resource
|
|
... provides shared setup/teardown, API key detection with
|
|
... graceful skip, and flexible assertion helpers.
|
|
Library OperatingSystem
|
|
Library String
|
|
Library Process
|
|
|
|
*** Variables ***
|
|
${WORKSPACE} ${CURDIR}/../..
|
|
${SRC_DIR} ${WORKSPACE}/src/cleveragents
|
|
${E2E_TEMP_ROOT} ${TEMPDIR}${/}cleveragents_e2e
|
|
|
|
*** Keywords ***
|
|
E2E Suite Setup
|
|
[Documentation] Set up the E2E test environment with per-suite isolation.
|
|
...
|
|
... Creates a unique CLEVERAGENTS_HOME directory.
|
|
... Does NOT enable mock AI — E2E tests use real providers.
|
|
... Propagates LLM API keys from the environment.
|
|
Log Setting up E2E test environment
|
|
${safe_suite}= Replace String ${SUITE NAME} ${SPACE} _
|
|
${safe_suite}= Replace String ${safe_suite} . _
|
|
${home}= Set Variable ${E2E_TEMP_ROOT}${/}${safe_suite}
|
|
${rm_status} ${rm_msg}= Run Keyword And Ignore Error Remove Directory ${home} recursive=True
|
|
IF '${rm_status}' == 'FAIL'
|
|
Log Could not remove previous suite home ${home}: ${rm_msg} WARN
|
|
END
|
|
Create Directory ${home}
|
|
Set Environment Variable CLEVERAGENTS_HOME ${home}
|
|
Set Suite Variable ${SUITE_HOME} ${home}
|
|
Set Environment Variable CLEVERAGENTS_AUTO_APPLY_MIGRATIONS true
|
|
# Explicitly disable mock AI — E2E tests use real providers
|
|
Remove Environment Variable CLEVERAGENTS_TESTING_USE_MOCK_AI
|
|
# Get the actual Python executable being used
|
|
${python_exec}= Evaluate sys.executable sys
|
|
Set Suite Variable ${PYTHON} ${python_exec}
|
|
|
|
E2E Suite Teardown
|
|
[Documentation] Clean up the E2E test environment.
|
|
Log Cleaning up E2E test environment
|
|
Run Keyword And Ignore Error Remove Directory ${SUITE_HOME} recursive=True
|
|
Remove Environment Variable CLEVERAGENTS_HOME
|
|
Remove Environment Variable CLEVERAGENTS_AUTO_APPLY_MIGRATIONS
|
|
|
|
Skip If No LLM Keys
|
|
[Documentation] Skip the current test if no LLM API keys are available.
|
|
...
|
|
... Checks for ANTHROPIC_API_KEY and OPENAI_API_KEY.
|
|
... If neither is set, the test is skipped gracefully.
|
|
... Keys are evaluated inline to avoid storing raw secrets
|
|
... in Robot Framework variables (which may be logged at DEBUG level).
|
|
${has_keys}= Evaluate bool(__import__('os').environ.get('ANTHROPIC_API_KEY', '')) or bool(__import__('os').environ.get('OPENAI_API_KEY', ''))
|
|
IF not ${has_keys}
|
|
Skip No LLM API keys available (ANTHROPIC_API_KEY / OPENAI_API_KEY). Skipping E2E test.
|
|
END
|
|
|
|
Run CleverAgents Command
|
|
[Documentation] Run a CleverAgents CLI command and return the result.
|
|
...
|
|
... Executes ``python -m cleveragents <args>`` using the
|
|
... venv Python. Runs in ``SUITE_HOME`` so the CLI creates
|
|
... its ``.cleveragents`` workspace inside the per-suite
|
|
... temp directory, avoiding cross-run database pollution.
|
|
... Returns the Process result object.
|
|
[Arguments] @{args} ${expected_rc}=${0} ${timeout}=120s ${cwd}=${SUITE_HOME}
|
|
${result}= Run Process ${PYTHON} -m cleveragents @{args}
|
|
... cwd=${cwd}
|
|
... timeout=${timeout}
|
|
... on_timeout=kill
|
|
... env:CLEVERAGENTS_HOME=${SUITE_HOME}
|
|
... env:CLEVERAGENTS_AUTO_APPLY_MIGRATIONS=true
|
|
... env:NO_COLOR=1
|
|
Log STDOUT: ${result.stdout} level=DEBUG
|
|
Log STDERR: ${result.stderr} level=DEBUG
|
|
IF '${expected_rc}' != 'None'
|
|
Should Be Equal As Integers ${result.rc} ${expected_rc}
|
|
... CleverAgents command failed with rc=${result.rc}. Check DEBUG-level log entries above.
|
|
END
|
|
RETURN ${result}
|
|
|
|
Output Should Contain
|
|
[Documentation] Assert that command output contains expected text (flexible).
|
|
...
|
|
... Checks stdout first, then stderr. Case-insensitive by default.
|
|
[Arguments] ${result} ${expected} ${case_insensitive}=${TRUE}
|
|
${combined}= Set Variable ${result.stdout}\n${result.stderr}
|
|
IF ${case_insensitive}
|
|
Should Contain ${combined.lower()} ${expected.lower()}
|
|
ELSE
|
|
Should Contain ${combined} ${expected}
|
|
END
|
|
|
|
Safe Parse Json Field
|
|
[Documentation] Safely extract a JSON field from CLI stdout.
|
|
...
|
|
... Tries two strategies to locate the JSON object:
|
|
... 1. Outer-bracket: first ``{`` to last ``}`` (works for
|
|
... single-object output with optional non-JSON preamble).
|
|
... 2. Last-line fallback: iterates lines from the end looking
|
|
... for a parseable JSON line (handles multi-object output
|
|
... where each object is on its own line).
|
|
... Returns empty string if no JSON is found, the field is
|
|
... absent, or all parse attempts fail.
|
|
[Arguments] ${stdout} ${field_name}
|
|
# Strategy 1: outer-bracket extraction (first '{' to last '}')
|
|
${pos}= Evaluate $stdout.find('{')
|
|
IF ${pos} == -1
|
|
RETURN ${EMPTY}
|
|
END
|
|
${end}= Evaluate $stdout.rfind('}')
|
|
IF ${end} == -1 or ${end} < ${pos}
|
|
RETURN ${EMPTY}
|
|
END
|
|
${json_str}= Evaluate $stdout[${pos}:${end}+1]
|
|
${status} ${value}= Run Keyword And Ignore Error
|
|
... Evaluate __import__('json').loads($json_str).get($field_name, '')
|
|
IF '${status}' == 'PASS'
|
|
RETURN ${value}
|
|
END
|
|
# Strategy 2: last-line fallback — scan lines in reverse for a parseable
|
|
# JSON object that contains the requested field.
|
|
${strategy1_err}= Set Variable ${value}
|
|
${strategy2_err}= Set Variable no JSON lines found
|
|
${lines}= Evaluate list(reversed([l.strip() for l in $stdout.splitlines() if l.strip().startswith('{') and l.strip().endswith('}')]))
|
|
FOR ${line} IN @{lines}
|
|
${ls} ${lv}= Run Keyword And Ignore Error
|
|
... Evaluate __import__('json').loads($line).get($field_name, '')
|
|
IF '${ls}' == 'PASS' and '${lv}' != ''
|
|
RETURN ${lv}
|
|
END
|
|
${strategy2_err}= Set Variable ${lv}
|
|
END
|
|
Log Safe Parse Json Field: JSON parse failed for field '${field_name}'. Strategy 1: ${strategy1_err}; Strategy 2: ${strategy2_err} WARN
|
|
RETURN ${EMPTY}
|
|
|
|
Create Temp Git Repo
|
|
[Documentation] Create a temporary git repository for E2E testing.
|
|
...
|
|
... Returns the path to the created repository.
|
|
[Arguments] ${name}=test-repo
|
|
${repo_dir}= Set Variable ${SUITE_HOME}${/}${name}
|
|
Create Directory ${repo_dir}
|
|
${r1}= Run Process git init cwd=${repo_dir} timeout=60s on_timeout=kill
|
|
Should Be Equal As Integers ${r1.rc} 0 msg=git init failed (rc=${r1.rc}). Check DEBUG logs above.
|
|
${r2}= Run Process git config user.name E2E Test cwd=${repo_dir} timeout=60s on_timeout=kill
|
|
Should Be Equal As Integers ${r2.rc} 0 msg=git config user.name failed
|
|
${r3}= Run Process git config user.email e2e@test.local cwd=${repo_dir} timeout=60s on_timeout=kill
|
|
Should Be Equal As Integers ${r3.rc} 0 msg=git config user.email failed
|
|
# Create an initial commit so the repo has a HEAD
|
|
Create File ${repo_dir}${/}README.md # Test Repository\n
|
|
${r4}= Run Process git add . cwd=${repo_dir} timeout=60s on_timeout=kill
|
|
Should Be Equal As Integers ${r4.rc} 0 msg=git add failed (rc=${r4.rc}). Check DEBUG logs above.
|
|
${r5}= Run Process git commit -m Initial commit cwd=${repo_dir} timeout=60s on_timeout=kill
|
|
Should Be Equal As Integers ${r5.rc} 0 msg=git commit failed (rc=${r5.rc}). Check DEBUG logs above.
|
|
RETURN ${repo_dir}
|