fix(testing): document and harden non-AssertionError guard in apply_tdd_inversion to reduce flaky CI #8325
Labels
No labels
auto/needs-reevaluation
controller-managed
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Blocks
#8294 Bug: @tdd_expected_fail scenarios fail intermittently when steps raise non-AssertionError exceptions, causing flaky CI
cleveragents/cleveragents-core
Reference
cleveragents/cleveragents-core!8325
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/tdd-inversion-non-assertion-guard"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
apply_tdd_inversion()surface non-AssertionErrorguard firings instandard Behave output by emitting the warning to both
stderrand thestructured logger via a new
_warning_with_stderrhelper.features/tdd_expected_fail_infrastructure.feature:a non-
AssertionErrorexception (e.g.RuntimeError) prevents inversion.exception IS an
AssertionError(correct expected-fail signal path), andconfirms the step is correctly set to
passedafter inversion.CONTRIBUTING.mdthat@tdd_expected_failbug-signaling failuresmust use
AssertionError(assert/raise AssertionError(...)) soinfrastructure/runtime exceptions are not accidentally treated as expected bug
failures.
CHANGELOG.mdentry for #8294 under[Unreleased] → ### Fixed.Approach
_warning_with_stderr(message: str)is a module-private helper that calls_tdd_logger.warning(message)andprint(message, file=sys.stderr). The callsite uses an f-string rather than lazy
%-style formatting; an inline commentdocuments this is intentional because the message is always emitted to
stderrvia
print()so deferred formatting provides no benefit. An additional inlinecomment in
_warning_with_stderrdocuments that the dual-emit is intentional:the logger may route to structured sinks not visible in CI output, so stderr
guarantees visibility in Behave's console output.
Exception text in the guard warning is defensively truncated to 500 characters
(
exc_text = str(step.exception)[:500]) to avoid runaway output for exceptionswith very long
str()representations.The
stderr output should not containBDD step validates the negative path usingredirect_stderr/StringIOcapture, the same mechanism as the positivescenario. The negative scenario now also asserts
And the step "bug step" should have status "passed"to match the positive scenario's step-level status coverageand make the asymmetry explicit.
Audit (AC #4)
An audit was performed of all existing
apply_tdd_inversioncallers and@tdd_expected_failscenarios across the codebase. No violations were found:all existing scenarios correctly use
AssertionError-based failures.Deferred items
_tdd_logger.warningon line ~238)was not updated to use
_warning_with_stderr; this is out of scope for #8294and will be addressed if it causes CI visibility issues (follow-up ticket
recommended).
_warning_with_stderr— acceptable for this PR.A dedicated unit test should be added in a follow-up ticket.
Validation
nox -e lint✓nox -e typecheck✓ (0 errors, 3 import-source warnings — pre-existing)nox -e unit_tests✓ (22 scenarios intdd_expected_fail_infrastructure.feature, all pass)nox -e integration_tests✓ (1873 Robot Framework tests passed, 0 failed; pabot exit 252 is a reporting artifact, not a test failure — confirmed via xunit.xml and output.xml)nox -e e2e_tests✓ (114 E2E tests passed, 0 failed; LLM API keys available in environment)nox(full suite) ✓ (lint, format, typecheck, security_scan, dead_code, unit_tests, integration_tests, docs, build, coverage_report — all sessions successful)nox -s coverage_report✓ (coverage: 97.25% ≥ 97% threshold — AC5 satisfied)Closes #8294
c237658e6d43106f951cThank you for submitting this PR, @hurui200320.
Acknowledgment:
PR #8325 (
fix(testing): harden TDD expected-fail non-assertion guard visibility) has been received. This PR addresses issue #8294 by:AssertionErrorguard warnings to stderr in addition to structured loggingAssertionErrorrequirement inCONTRIBUTING.mdStatus: This PR is open and awaiting review. The automated review pipeline will assess it against the project's quality gates. A human or automated review will follow.
If you have any questions about the review process or timeline, please comment here.
Automated by CleverAgents Bot
Supervisor: Human Liaison | Agent: human-liaison-pool-supervisor
pr-creator(PRs missing required labels) #8520HAL9000 referenced this pull request2026-04-13 21:28:54 +00:00
[GROOMED]
Quality issues identified:
Actions taken:
Outstanding follow-ups:
Automated by CleverAgents Bot
Supervisor: Grooming Pool | Agent: grooming-pool-supervisor
Worker: [AUTO-GROOM-8325]
43106f951c854f608a21854f608a2174c6d2651bfix(testing): harden TDD expected-fail non-assertion guard visibilityto fix(testing): document and harden non-AssertionError guard in apply_tdd_inversion to reduce flaky CI@HAL9001 please review this PR
@HAL9000 please review this PR
[GROOMED]
Summary:
Automated by CleverAgents Bot
Supervisor: Grooming | Agent: grooming-pool-supervisor
Code Review: APPROVED ✅
Summary
This PR correctly addresses all acceptance criteria from issue #8294. The implementation is clean, well-documented, and all quality gates pass.
Checklist
fix(testing): ...withISSUES CLOSED: #8294footerCloses #8294in PR descriptionType/*labelType/Bug[Unreleased] → ### Fixedtdd_expected_fail_infrastructure.featurenox -s coverage_reportImplementation Review
features/environment.py—_warning_with_stderrhelper%-style to f-string is intentional and correctly documented with an inline comment.message: str) -> None) is correct.features/steps/tdd_expected_fail_infrastructure_steps.py— new step definitionsstep_run_apply_inversion_failed_capture_stderrcorrectly usescontextlib.redirect_stderr+StringIOfor clean, side-effect-free stderr capture.step_check_stderr_containsandstep_check_stderr_not_containsboth guard against missingcontext.captured_stderrwith a clear error message pointing to the correct When step.features/tdd_expected_fail_infrastructure.feature— new BDD scenariosapply_tdd_inversion surfaces non-AssertionError guard warning to stderr): correctly asserts guard fires, result isTrue(not inverted), and stderr contains the expected warning text.apply_tdd_inversion does not emit guard warning for AssertionError exceptions): correctly asserts no warning emitted, result isFalse(inverted), and step status ispassed— providing symmetric coverage to the positive path.CHANGELOG.md— entry is accurate, well-scoped, and placed correctly under[Unreleased] → ### Fixed.CONTRIBUTING.md— the new rule is clear, actionable, and placed in the correct section (TDD Issue Test Tags).Deferred Items (Acknowledged)
The PR explicitly documents two acceptable deferrals:
_tdd_logger.warning~line 238) not updated — out of scope for #8294._warning_with_stderr— acceptable; follow-up ticket recommended.Neither deferred item blocks this PR.
Verdict
All five acceptance criteria from issue #8294 are satisfied. The code is correct, well-tested, and does not introduce regressions. Approved for merge.
Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
Worker: [AUTO-REV-8325]
Code Review Decision: APPROVED ✅
All quality criteria satisfied. PR #8325 correctly implements all five acceptance criteria from issue #8294:
_warning_with_stderrhelper emits guard warnings to both structured logger andstderrAssertionError-only requirement for@tdd_expected_failstepstdd_expected_fail_infrastructure.feature(positive + negative paths)@tdd_expected_failstep definitions — no violations foundAll 13 CI jobs green. Commit message follows Conventional Commit format with
ISSUES CLOSED: #8294footer. Milestone v3.2.0 andType/Buglabel correctly set.Automated by CleverAgents Bot
Supervisor: PR Review Pool | Agent: pr-reviewer
Worker: [AUTO-REV-8325]
74c6d2651bed7dba552c@hurui200320 — The automated review pool has approved PR #8325. All five acceptance criteria from issue #8294 have been verified as satisfied:
_warning_with_stderrhelper correctly emits guard warnings to both structured logger and stderrAssertionError-only requirement for@tdd_expected_failstepstdd_expected_fail_infrastructure.feature(positive and negative paths)@tdd_expected_failstep definitions confirmed no violationsAll 13 CI jobs are green. Commit message follows Conventional Commit format with
ISSUES CLOSED: #8294footer. Milestone v3.2.0 andType/Buglabel are correctly set.The merge pool will process this PR for merging. Thank you for the contribution.
Automated by CleverAgents Bot
Supervisor: Human Liaison | Agent: human-liaison-pool-supervisor
Worker: [AUTO-HUMAN-8]
HAL9000 referenced this pull request2026-04-15 03:17:32 +00:00
ed7dba552cf67e8a2e07