fix(test): fix tolerant exit code and missing RC check in resource CLI test #907

Merged
freemo merged 1 commit from fix/integration-resource-cli-tolerant-rc into master 2026-03-14 19:55:33 +00:00
Owner

Summary

Fixes #896

  • Changed resource type list exit code check from rc == 0 or rc == 1 to strict rc == 0 — the test should fail if the command returns an error
  • Added return code assertion to Suite Setup database schema creation — if schema creation fails, all subsequent tests would pass for wrong reasons
  • Broadened exception handling in all 14 resource CLI command handlers (resource.py) with except Exception catch-all blocks so unhandled exceptions produce a clean "Unexpected error" message and Aborted! exit instead of raw tracebacks
  • Added isinstance guard to re-raise typer.Abort and typer.Exit in the catch-all handlers — these are subclasses of Exception (via click.exceptions) and must propagate normally for typer's exit machinery to work
  • Replaced Unicode arrow characters () with ASCII (->) in resource_stop and resource_rebuild docstrings to avoid encoding issues

Part of Epic #892.

## Summary Fixes #896 - Changed `resource type list` exit code check from `rc == 0 or rc == 1` to strict `rc == 0` — the test should fail if the command returns an error - Added return code assertion to Suite Setup database schema creation — if schema creation fails, all subsequent tests would pass for wrong reasons - Broadened exception handling in all 14 resource CLI command handlers (`resource.py`) with `except Exception` catch-all blocks so unhandled exceptions produce a clean "Unexpected error" message and `Aborted!` exit instead of raw tracebacks - Added `isinstance` guard to re-raise `typer.Abort` and `typer.Exit` in the catch-all handlers — these are subclasses of `Exception` (via `click.exceptions`) and must propagate normally for typer's exit machinery to work - Replaced Unicode arrow characters (`→`) with ASCII (`->`) in `resource_stop` and `resource_rebuild` docstrings to avoid encoding issues Part of Epic #892.
freemo force-pushed fix/integration-resource-cli-tolerant-rc from 9f86ea4e66
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 15s
CI / quality (pull_request) Successful in 22s
CI / build (pull_request) Successful in 21s
CI / security (pull_request) Successful in 33s
CI / e2e_tests (pull_request) Successful in 27s
CI / typecheck (pull_request) Successful in 37s
CI / unit_tests (pull_request) Has been cancelled
CI / integration_tests (pull_request) Has been cancelled
CI / coverage (pull_request) Has been cancelled
CI / benchmark-regression (pull_request) Has been cancelled
CI / docker (pull_request) Has been cancelled
to f2b8580e15
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 19s
CI / build (pull_request) Successful in 18s
CI / quality (pull_request) Successful in 21s
CI / e2e_tests (pull_request) Successful in 31s
CI / security (pull_request) Successful in 38s
CI / typecheck (pull_request) Successful in 42s
CI / unit_tests (pull_request) Successful in 2m10s
CI / integration_tests (pull_request) Successful in 2m40s
CI / docker (pull_request) Successful in 43s
CI / coverage (pull_request) Successful in 6m5s
CI / benchmark-regression (pull_request) Failing after 19m59s
2026-03-13 23:56:20 +00:00
Compare
freemo force-pushed fix/integration-resource-cli-tolerant-rc from c3a153f2df
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 15s
CI / build (pull_request) Successful in 16s
CI / quality (pull_request) Successful in 18s
CI / e2e_tests (pull_request) Successful in 23s
CI / security (pull_request) Successful in 32s
CI / typecheck (pull_request) Successful in 39s
CI / integration_tests (pull_request) Successful in 2m41s
CI / unit_tests (pull_request) Successful in 2m52s
CI / docker (pull_request) Successful in 53s
CI / coverage (pull_request) Successful in 5m6s
CI / benchmark-regression (pull_request) Has been cancelled
to e580f57b60
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 15s
CI / build (pull_request) Successful in 15s
CI / quality (pull_request) Successful in 17s
CI / e2e_tests (pull_request) Successful in 27s
CI / security (pull_request) Successful in 31s
CI / typecheck (pull_request) Successful in 33s
CI / unit_tests (pull_request) Successful in 2m6s
CI / docker (pull_request) Successful in 9s
CI / integration_tests (pull_request) Failing after 4m35s
CI / coverage (pull_request) Successful in 6m0s
CI / benchmark-regression (pull_request) Successful in 35m58s
2026-03-14 03:04:35 +00:00
Compare
freemo scheduled this pull request to auto merge when all checks succeed 2026-03-14 03:04:47 +00:00
freemo added this to the v3.2.0 milestone 2026-03-14 04:34:40 +00:00
Author
Owner

PM Review — Day 34

Status: Mergeable, 0 reviews, Priority/Critical, M3 (v3.2.0)
Fixes: #896 (assigned to @brent.edwards) | Part of: Epic #892

Review Summary

The Robot Framework test changes are correct and well-scoped:

  • Suite Setup RC assertion: Previously the DB schema creation result was discarded — schema failures would silently let all downstream tests pass against a broken database. Now fails fast. ✓
  • Strict RC check: rc == 0 or rc == 1rc == 0. Eliminates masking of real CLI errors. ✓

Scope Concern

The third commit (e580f57) adds 14 catch-all except Exception handlers to src/cleveragents/cli/commands/resource.pyproduction code not mentioned in the PR title or description. This is scope creep:

  • The PR title says "fix(test)" but commit 3 is "fix(cli)"
  • The exception handlers are reasonable (clean error UX for CLI users), but should be documented in the PR body
  • Consider adding logger.exception() alongside the console print to preserve debuggability

Action Items

Who Action Deadline
@freemo Update PR description to document the resource.py exception handling changes Day 35
@brent.edwards Review this PR — you're the assignee on parent issue #896 and the QA lead Day 35

Labels Applied This Session

  • Type/Testing, Priority/Critical, State/In Review, MoSCoW/Must have
  • Milestone set to v3.2.0 (matching parent issue #896)
## PM Review — Day 34 **Status**: Mergeable, 0 reviews, Priority/Critical, M3 (v3.2.0) **Fixes**: #896 (assigned to @brent.edwards) | **Part of**: Epic #892 ### Review Summary The Robot Framework test changes are correct and well-scoped: - **Suite Setup RC assertion**: Previously the DB schema creation result was discarded — schema failures would silently let all downstream tests pass against a broken database. Now fails fast. ✓ - **Strict RC check**: `rc == 0 or rc == 1` → `rc == 0`. Eliminates masking of real CLI errors. ✓ ### Scope Concern The third commit (`e580f57`) adds 14 catch-all `except Exception` handlers to `src/cleveragents/cli/commands/resource.py` — **production code not mentioned in the PR title or description**. This is scope creep: - The PR title says "fix(test)" but commit 3 is "fix(cli)" - The exception handlers are reasonable (clean error UX for CLI users), but should be documented in the PR body - Consider adding `logger.exception()` alongside the console print to preserve debuggability ### Action Items | Who | Action | Deadline | |-----|--------|----------| | @freemo | Update PR description to document the `resource.py` exception handling changes | Day 35 | | @brent.edwards | **Review this PR** — you're the assignee on parent issue #896 and the QA lead | Day 35 | ### Labels Applied This Session - Type/Testing, Priority/Critical, State/In Review, MoSCoW/Must have - Milestone set to v3.2.0 (matching parent issue #896)
freemo force-pushed fix/integration-resource-cli-tolerant-rc from 4121521893
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 14s
CI / build (pull_request) Successful in 16s
CI / quality (pull_request) Successful in 21s
CI / e2e_tests (pull_request) Successful in 25s
CI / security (pull_request) Successful in 34s
CI / typecheck (pull_request) Successful in 38s
CI / unit_tests (pull_request) Successful in 2m15s
CI / integration_tests (pull_request) Successful in 2m50s
CI / docker (pull_request) Successful in 40s
CI / coverage (pull_request) Successful in 5m15s
CI / benchmark-regression (pull_request) Successful in 34m27s
to a76166909f
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / typecheck (pull_request) Failing after 21s
CI / lint (pull_request) Failing after 21s
CI / quality (pull_request) Failing after 18s
CI / coverage (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Has been skipped
CI / build (pull_request) Failing after 19s
CI / e2e_tests (pull_request) Failing after 29s
CI / unit_tests (pull_request) Failing after 30s
CI / security (pull_request) Failing after 34s
CI / docker (pull_request) Has been skipped
CI / integration_tests (pull_request) Failing after 38s
2026-03-14 16:09:13 +00:00
Compare
freemo force-pushed fix/integration-resource-cli-tolerant-rc from a76166909f
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / typecheck (pull_request) Failing after 21s
CI / lint (pull_request) Failing after 21s
CI / quality (pull_request) Failing after 18s
CI / coverage (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Has been skipped
CI / build (pull_request) Failing after 19s
CI / e2e_tests (pull_request) Failing after 29s
CI / unit_tests (pull_request) Failing after 30s
CI / security (pull_request) Failing after 34s
CI / docker (pull_request) Has been skipped
CI / integration_tests (pull_request) Failing after 38s
to 2e3c48071b
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 43s
CI / quality (pull_request) Successful in 43s
CI / typecheck (pull_request) Successful in 54s
CI / security (pull_request) Successful in 1m2s
CI / e2e_tests (pull_request) Successful in 45s
CI / build (pull_request) Successful in 45s
CI / benchmark-regression (pull_request) Has been cancelled
CI / integration_tests (pull_request) Has been cancelled
CI / unit_tests (pull_request) Has been cancelled
CI / coverage (pull_request) Has been cancelled
CI / docker (pull_request) Has been cancelled
2026-03-14 19:46:05 +00:00
Compare
freemo force-pushed fix/integration-resource-cli-tolerant-rc from 2e3c48071b
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 43s
CI / quality (pull_request) Successful in 43s
CI / typecheck (pull_request) Successful in 54s
CI / security (pull_request) Successful in 1m2s
CI / e2e_tests (pull_request) Successful in 45s
CI / build (pull_request) Successful in 45s
CI / benchmark-regression (pull_request) Has been cancelled
CI / integration_tests (pull_request) Has been cancelled
CI / unit_tests (pull_request) Has been cancelled
CI / coverage (pull_request) Has been cancelled
CI / docker (pull_request) Has been cancelled
to ec450e9085
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 34s
CI / quality (pull_request) Successful in 34s
CI / build (pull_request) Successful in 24s
CI / typecheck (pull_request) Successful in 40s
CI / e2e_tests (pull_request) Successful in 29s
CI / security (pull_request) Successful in 1m10s
CI / unit_tests (pull_request) Successful in 5m27s
CI / integration_tests (pull_request) Successful in 5m41s
CI / coverage (pull_request) Successful in 5m56s
CI / docker (pull_request) Successful in 1m9s
CI / lint (push) Successful in 22s
CI / quality (push) Successful in 31s
CI / e2e_tests (push) Successful in 50s
CI / typecheck (push) Successful in 56s
CI / security (push) Successful in 57s
CI / benchmark-regression (push) Has been skipped
CI / build (push) Successful in 41s
CI / unit_tests (push) Successful in 3m36s
CI / integration_tests (push) Successful in 3m46s
CI / docker (push) Successful in 1m7s
CI / coverage (push) Successful in 6m57s
CI / benchmark-publish (push) Successful in 19m47s
CI / benchmark-regression (pull_request) Successful in 44m27s
2026-03-14 19:48:46 +00:00
Compare
freemo merged commit ec450e9085 into master 2026-03-14 19:55:33 +00:00
freemo deleted branch fix/integration-resource-cli-tolerant-rc 2026-03-14 19:55:33 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core!907
No description provided.