chore(tests): suppress passing scenario output by default in behave-parallel unit test runner #10988
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
#10987 Suppress passing BDD scenario output in unit_tests session by default to reduce CI noise
cleveragents/cleveragents-core
Reference
cleveragents/cleveragents-core!10988
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/test-infra/suppress-passing-output"
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
This PR has three commits:
49f1cfc— ImplementsPassSuppressFormatter, a custom Behave formatter that eliminates the ~100,000-line noise produced by passing scenarios duringnox -s unit_tests. An all-passing suite now produces ≤ 10 lines of output (the_print_overall_summaryblock only).9acad7b— Fixes five pre-existing BDD unit test failures on master that were discovered during pipeline validation of commit 1. These failures are unrelated to the formatter change and are fixed in a separate commit to preserve commit atomicity.1d528f7— Fixes six integration test failures (Robot.Behave Parallel Log Filtering) caused by aModuleNotFoundErrorforPassSuppressFormatterwhen the integration test helper loadsscripts/run_behave_parallel.pyvia importlib outside of a nox session. The fix addsscripts/tosys.pathin the helper so the top-level import resolves correctly. Also addresses review feedback: reducesscripts/run_behave_parallel.pyto 499 lines (under the 500-line limit) and moves in-functionbehaveimports in the steps file to the top level.Motivation
The
unit_testssession runs ~15,230 BDD scenarios across 682 feature files. Every passing scenario printed its name, steps, and result unconditionally. This made it essentially impossible for both humans and AI agents to find failing scenarios in CI logs without parsing thousands of irrelevant lines.Approach
PassSuppressFormatter(inscripts/behave_pass_suppress_formatter.py)Formatterbase class sobehave.formatter._registry.register_as()accepts it.io.StringIO. At scenario end (nextscenario()call oreof()), the buffer is flushed to the real output stream only ifscenario.status.name not in {"passed", "skipped"}.Integration in
_make_runner()Runneris created.config.formatisNone(no explicit-f/--format) andBEHAVE_PARALLEL_COVERAGEis not set: usespass_suppressas the format.config.default_formatso slipcover can instrument a single process without output interference.Files Changed
Feature commit (
49f1cfc)scripts/behave_pass_suppress_formatter.pyPassSuppressFormatterclassscripts/run_behave_parallel.py_make_runner()features/behave_parallel_log_filtering.featurefeatures/steps/behave_parallel_log_filtering_steps.pynoxfile.pyCHANGELOG.mdPre-existing unit test fix commit (
9acad7b)architecture.feature:37— Type hints throughoutIndexEntry/ACMSIndexused@dataclasswithout PydanticBaseModelIndexEntrytoBaseModel,ACMSIndexto plain class insrc/cleveragents/acms/index.pypr_compliance_checklist.feature— All 10 scenariosPROJECT_ROOTusedparents[3]which resolved to/instead of workspace root/appparents[2]infeatures/steps/pr_compliance_checklist_steps.pyacms/index_data_model_and_traversal.feature— Scenarios 1 & 16| key | value |/| filter | value |Gherkin header rows causedKeyErrorfeatures/acms/index_data_model_and_traversal.featurecli_init_yes_flag.feature— All 5 scenarios (cleanup error)_restore_cwd()calledshutil.rmtree(context.temp_dir)whentemp_dirwasNonegetattrguard infeatures/steps/cli_init_yes_flag_steps.pysecurity_audit.feature:114,119— Count scenarios"the count should be {count:d}"shadowed security_audit's identical pattern, causingAttributeErroroncontext.entry_count"the ACMS entry count should be {count:d}"in bothfeatures/steps/acms_index_data_model_traversal_steps.pyandfeatures/acms/index_data_model_and_traversal.featureIntegration test fix + review feedback commit (
1d528f7)robot/helper_behave_parallel_log_filtering.pysys.path.insert(0, scripts_dir)before loadingrun_behave_parallel.pyvia importlibscripts/run_behave_parallel.pyfeatures/steps/behave_parallel_log_filtering_steps.pyConfigurationandStreamOpenerimports to top-level blockCloses #10987
Implemented PassSuppressFormatter, a custom Behave formatter that buffers per-scenario output and only flushes it to stdout when the scenario failed or errored. Passing scenarios produce no output, keeping an all-passing suite at ~5-10 lines (the _print_overall_summary block only). Key design decisions: - PassSuppressFormatter inherits from behave's Formatter base class so it can be registered via behave.formatter._registry.register_as(), which enforces issubclass(cls, Formatter). - _SUPPRESS_STATUSES = {'passed', 'skipped'} determines which terminal statuses are silently discarded; all others (failed, undefined, etc.) trigger a buffer flush so no failure is hidden. - _make_runner() registers the formatter and sets it as the default format whenever config.format is None (no explicit -f/--format flag). Coverage mode (BEHAVE_PARALLEL_COVERAGE=1) bypasses the formatter and falls back to config.default_format so slipcover can instrument a single process. - The formatter is embedded directly in run_behave_parallel.py (not a separate file) so the noxfile's _install_behave_parallel() packaging step, which copies only run_behave_parallel.py, picks it up with zero noxfile changes. - Three new BDD scenarios in behave_parallel_log_filtering.feature cover: (1) passing scenario -> no output, (2) failing scenario -> full output, (3) mixed run -> only failing scenario visible. All 20 scenarios pass. ISSUES CLOSED: #10987Review Summary
Thank you for implementing
PassSuppressFormatter— the core idea is sound and well-executed. The formatter logic, buffering strategy, and coverage-mode bypass are all correct. The three new BDD scenarios appropriately test the formatter in isolation. However, there are three blocking issues that must be resolved before this PR can be merged.Blocking Issues
1. CI
unit_testsis FailingThe
CI / unit_testsjob is failing ("Failing after 5m45s"). Per company policy, all required CI gates must pass before a PR can be approved and merged. A passingunit_testssuite is a mandatory merge gate.The PR description states that unit_tests passed locally (20/20 scenarios for the target feature), but CI is reporting a failure on the full suite. This discrepancy must be investigated and fixed. Possible causes: the new formatter is interfering with scenario output collection in a way not caught by the 3 isolated unit tests, a regression in existing scenarios, or an environment-specific issue.
Action required: Diagnose the CI
unit_testsfailure and push a fix. The full suite must be green before this PR can be merged.2. CHANGELOG Not Updated
The
CHANGELOG.mdhas not been updated to include an entry for this change. Per the CONTRIBUTING.md checklist, every PR must include a changelog entry describing the change for users. This is item 7 of the 12 pre-submission requirements and is a merge gate.Action required: Add an entry to the
[Unreleased]section ofCHANGELOG.mdunder the appropriate heading (e.g.### Changed) describing the newPassSuppressFormatterbehaviour and how it reduces CI log noise.3.
scripts/run_behave_parallel.pyExceeds the 500-Line Limitscripts/run_behave_parallel.pynow has 621 lines (up from 485 onmaster). The project's code style rule requires files to remain under 500 lines. Notably, the issue's own Metadata and Subtasks section anticipated this: it suggested placing the formatter inscripts/behave_pass_suppress_formatter.py(orfeatures/formatters/pass_suppress.py). The PR explanation correctly identifies the embedding rationale (the noxfile copies onlyrun_behave_parallel.py), but that is an argument for reconsidering the noxfile's_install_behave_parallel()packaging step — not for violating the 500-line rule.Action required: Extract
PassSuppressFormatterand_SUPPRESS_STATUSESinto a separate file (e.g.scripts/behave_pass_suppress_formatter.py). Update_install_behave_parallel()innoxfile.pyto copy both files, or updaterun_behave_parallel.pyto import from the new module. Either path keeps both files under 500 lines and satisfies the issue's own subtask guidance.Non-Blocking Observations (Suggestions)
Branch Naming Convention Deviation
The branch
feat/test-infra/suppress-passing-outputdoes not follow the required naming patternfeature/mN-<descriptive-name>. For milestone v3.2.0 (m3), it should befeature/m3-suppress-passing-output. The issue's Metadata section prescribes this exact non-conforming name, which means the issue itself was authored with a non-compliant branch name. This does not block the PR since the branch name matches the issue's Metadata verbatim, but it is noted for awareness.type: ignore[import-untyped]AdditionsThree new
# type: ignore[import-untyped]annotations are added forbehaveimports (Formatter,Configuration,StreamOpener). These are consistent with the pre-existing pattern already in the same files (behave has no type stubs). The project prohibits# type: ignorefor suppressing actual type errors in production code, but[import-untyped]for untyped third-party libraries is the accepted pattern here. No change needed.In-Function Imports in
_make_pass_suppress_formatter()The helper in the steps file uses
from behave.configuration import Configurationandfrom behave.formatter.base import StreamOpenerinside the function body. The project rule requires all imports at the top of the file. However,_make_runner()inrun_behave_parallel.pyhas used in-function behave imports since before this PR (from behave.configuration import Configuration,from behave.runner import Runner, etc. at lines 276–278 on master). This is a pre-existing deviation; the new code follows the established local pattern. Suggest a follow-up issue to consolidate all behave imports to the top level to align with the import rules.Acceptance Criteria Discrepancy on Output Line Count
The issue states "≤ 30 lines of output" for an all-passing suite; the PR description says "≤ 10 lines". This is a minor documentation inconsistency — the implementation exceeds the stricter bar, which is fine. No action needed, but worth aligning in a follow-up.
What's Working Well
io.StringIO, flushing only on non-suppress statuses, and resetting the buffer on eachscenario()call and ateof()is the right approach.BEHAVE_PARALLEL_COVERAGEenv var check correctly falls back to the default format so slipcover is unaffected.register_as()beforeRunnercreation ensures behave can resolve the format name atmake_formatters()time._run_features_inprocess()and_worker_run_features()call_make_runner(), so the formatter applies in both execution paths.ISSUES CLOSED: #10987✅lint,typecheck,security,integration_tests,e2e_tests,buildall passing ✅Please address the three blocking issues and push a new commit. I will re-review once CI is green.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
BLOCKING — CHANGELOG entry missing
This PR does not add a
CHANGELOG.mdentry. Per CONTRIBUTING.md item 7, every PR must include a changelog entry describing the change for users.How to fix: Add an entry to the
[Unreleased]section, for example:@ -34,0 +34,4 @@from behave.formatter.base import (Formatter as _BehaveFormatter, # type: ignore[import-untyped])BLOCKING — File exceeds 500-line limit
This import of
_BehaveFormatteris the start of the newPassSuppressFormatterclass, which pushesrun_behave_parallel.pyfrom 485 lines (onmaster) to 621 lines — 121 lines over the 500-line limit.The issue's own Subtasks section anticipated this and suggested extracting the formatter to a dedicated file:
How to fix: Extract
_SUPPRESS_STATUSESandPassSuppressFormatter(lines 38–151 of the new file) intoscripts/behave_pass_suppress_formatter.py, thenfrom behave_pass_suppress_formatter import PassSuppressFormatter, _SUPPRESS_STATUSESat the top ofrun_behave_parallel.py. Update_install_behave_parallel()innoxfile.pyto copy both script files.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
e20cfa3ebf49f1cfcdb6Re-Review Summary
Thank you for the follow-up commit addressing the prior feedback. Two of the three blocking issues from the previous review have been fully or substantially resolved, and the overall implementation quality is high. However, two blocking issues remain — one carryover and one new — that must be resolved before this PR can be approved.
Status of Prior Blocking Issues
✅ Blocking Issue #2 — RESOLVED: CHANGELOG Updated
A proper
[Unreleased]entry has been added toCHANGELOG.mddescribingPassSuppressFormatterand its impact. The entry is clear, accurate, and appropriately placed. This issue is fully resolved.✅ Partially Resolved: File Size (run_behave_parallel.py)
The formatter was correctly extracted to
scripts/behave_pass_suppress_formatter.py(125 lines), andnoxfile.py's_install_behave_parallel()was updated to copy both files — exactly as requested. However,scripts/run_behave_parallel.pyis now exactly 500 lines, and the project rule requires files to be under 500 lines (i.e. ≤ 499). This is technically still a 1-line violation. Removing 1 line from the script (e.g. collapsing the blank separator line between thetry/exceptblock andDEFAULT_FEATURE_ROOT, or tightening one of the reformatted docstrings) would resolve this.Action required: Reduce
scripts/run_behave_parallel.pyto ≤ 499 lines.New Blocking Issues
1. CI
integration_testsIs Newly FailingThe
CI / integration_tests (pull_request)check is failing ("Failing after 3m28s") on this PR's head commit (49f1cfcd). On the base commit (f2d1f4ef/ master), the same pull_request context showsintegration_testsas passing. This means the integration test failure was introduced by this PR.Note: The
CI / unit_testsfailure is pre-existing onmaster(master's own pull_request CI also shows unit_tests failing) and therefore cannot be attributed to this PR. TheCI / benchmark-regressionfailure is also pre-existing on master in the pull_request context. However,integration_testsis a new regression.This PR touches
scripts/run_behave_parallel.py,noxfile.py, andfeatures/— the noxfile change (addingformatter_scriptto_install_behave_parallel()) could affect how thebehave_parallelpackage is installed and therefore impact test execution if any integration tests invoke theunit_testsnox session or the behave-parallel runner indirectly.Action required: Diagnose the
CI / integration_testsfailure. If it is caused by changes in this PR, fix it. If it is an unrelated transient failure, re-trigger CI to confirm.Non-Blocking Observations
In-Function Imports in
_make_pass_suppress_formatter()(Steps File)The helper
_make_pass_suppress_formatter()infeatures/steps/behave_parallel_log_filtering_steps.pyadds two in-function imports:from behave.configuration import Configurationfrom behave.formatter.base import StreamOpenerThe prior review noted this same pattern in
_make_runner()as a pre-existing deviation. New code should not perpetuate the violation — the import rule requires all imports at the top of the file. Suggest moving these two imports to the top-level# type: ignore[import-untyped]block alongside the existingbehaveimports.What's Working Well (Confirmed in This Review)
scripts/behave_pass_suppress_formatter.pyat 125 lines is clean, well-documented, and properly structured. The module docstring,_SUPPRESS_STATUSESconstant, and all lifecycle method implementations are correct.try: from behave_pass_suppress_formatter import ... except ImportError: from behave_parallel...correctly handles both the direct-script path and the installed-package path without violating in-function import rules.cli.pyandbehave_pass_suppress_formatter.pyare now copied into thebehave_parallelpackage directory.PassSuppressFormatterscenarios inbehave_parallel_log_filtering.featurecorrectly test passing-suppression, failure-emission, and mixed-run behaviours.ISSUES CLOSED: #10987✅Please address the two blocking issues (500-line boundary and integration_tests CI failure) and push a new commit. I will re-review promptly.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@ -34,1 +34,4 @@PassSuppressFormatter,)DEFAULT_FEATURE_ROOT = "features/"BLOCKING — File is at exactly 500 lines (rule requires under 500)
scripts/run_behave_parallel.pyis currently 500 lines exactly. The project code style rule requires files to be under 500 lines (≤ 499 lines). While this is improved from the previous submission, it remains technically non-compliant by 1 line.Options to resolve with zero functional change:
try/exceptblock andDEFAULT_FEATURE_ROOT = "features/"(line 35).Any one of these reduces the file to ≤ 499 lines.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Why Two Commits?
This PR now has two commits:
49f1cfc—chore(tests): suppress passing scenario output...— The original feature commit implementingPassSuppressFormatter. This commit is atomic and self-contained: it only touches the output suppression feature and its tests.9acad7b—fix(tests): resolve pre-existing unit test failures...— A follow-up commit fixing five pre-existing BDD test failures that were already broken onmasterbefore this PR was opened.Why Not Amend?
The five test failures are not caused by the PassSuppressFormatter change. They are pre-existing issues on master:
architecture.feature:37IndexEntry/ACMSIndexused@dataclasswithout PydanticBaseModelpr_compliance_checklist.feature(all 10)PROJECT_ROOTusedparents[3]which resolved to/instead of/appacms/index_data_model_and_traversal.feature:10,126| key | value |/| filter | value |)cli_init_yes_flag.feature(all 5)_restore_cwd()calledshutil.rmtree(context.temp_dir)whentemp_dirwasNonesecurity_audit.feature:114,119"the count should be {count:d}"shadowed security_audit's identical patternAdding these fixes to the feature commit would violate commit atomicity — the feature commit would no longer be about "suppress passing scenario output" and would instead mix unrelated test fixes into a single commit. Keeping them separate makes each commit reviewable independently and preserves the project's one-commit-per-issue discipline.
Also why not create a separate PR to fix pre-existing issues?
Thus, fixing broken tests in this PR seems to be the easiest and most effective move.
Since the PR contains fix to the CI pipeline, I have set the label CI Blocker.
Addressed the
CI / integration_testsfailure.Root cause: The integration test helper (
robot/helper_behave_parallel_log_filtering.py) loadsscripts/run_behave_parallel.pyvia importlib. This triggers the top-levelfrom behave_pass_suppress_formatter import PassSuppressFormatterimport in the runner module. When invoked outside of a nox session (i.e. during integration tests), neitherbehave_pass_suppress_formatternor thebehave_parallelpackage (created bynoxfile.pyforunit_tests) is onsys.path, causingModuleNotFoundErrorand all 6 Robot tests to fail with exit code 1.Fix (commit
173d1fb): Addedscripts/tosys.pathin the helper's_load_runner_module()before loading the runner module, sofrom behave_pass_suppress_formatter import PassSuppressFormatterresolves correctly. This mirrors the approach already used innoxfile.py'sunit_testssession for thebehave-parallelpackage.Verification: All 6 integration tests now pass (
6 tests, 6 passed, 0 failed, 0 skipped), and all 1986 integration tests pass without regression.Regarding the 500-line boundary on
scripts/run_behave_parallel.py: this is a pre-existing issue from the first commit (49f1cfc) and is outside the scope of the current integration test fix. Theunit_testscommit (9acad7b) was explicitly not to be amended per the fix instructions.Re-Review Summary
Thank you for the two new commits addressing the prior feedback. Significant progress has been made:
unit_testsandintegration_testsare now both green, CHANGELOG is updated, and the formatter extraction to a separate file is correct. However, one blocking issue remains from the previous review — plus two new blocking issues introduced by the additional commits.Status of Prior Blocking Issues
✅ RESOLVED: CI
unit_testsFailureCommit
9acad7bcorrectly fixes five pre-existing BDD test failures on master. All five root causes are correctly diagnosed and resolved:IndexEntryconverted to PydanticBaseModel✅PROJECT_ROOTpath fixed (parents[2]) ✅getattrguard oncontext.temp_dir✅CI / unit_testsis now passing. ✅✅ RESOLVED: CI
integration_testsFailureCommit
173d1fbcorrectly addsscripts/tosys.pathin_load_runner_module()before loading the runner via importlib.CI / integration_testsis now passing. ✅✅ RESOLVED: CHANGELOG Updated
The
[Unreleased]entry is clear, accurate, and correctly describes the formatter behaviour. ✅🔴 STILL BLOCKING:
scripts/run_behave_parallel.pyIs Exactly 500 Linesscripts/run_behave_parallel.pyis at exactly 500 lines. The project rule requires files to be under 500 lines (≤ 499). This has been flagged in both previous reviews and remains unresolved.The fix is trivial — removing any single blank line reduces the file to 499 lines with zero functional change.
Action required: Remove one blank line from
scripts/run_behave_parallel.pyto bring it to ≤ 499 lines.New Blocking Issues
1. Commits
9acad7band173d1fbAre Missing Issue Reference FootersBoth new commits lack the required
ISSUES CLOSED: #NorRefs: #Nfooter. CONTRIBUTING.md requires every commit footer to include an issue reference.9acad7b(fix(tests): resolve pre-existing unit test failures in 5 feature files): No footer present. At minimum, addRefs: #10987since fixing these failures is a prerequisite for this PR.173d1fb(fix(tests): resolve integration test failures in behave parallel log filtering): No footer present. AddRefs: #10987or a reference to the relevant integration test issue.Action required: Add
Refs: #10987(orISSUES CLOSED: #Nif dedicated issues exist) to the footer of each of these two commits via interactive rebase before merge.2. In-Function Imports in
_make_pass_suppress_formatter()Violate the Import RuleLines 406–407 in
features/steps/behave_parallel_log_filtering_steps.pyplace two imports inside the function body:The project rule requires all imports at the top of the file, with the sole exception of
if TYPE_CHECKING:blocks. This file already has a correct top-level behave import block at lines 18–19. The pre-existing in-function import pattern inrun_behave_parallel.pydoes not justify a new violation here — that file is a known deviation; the steps file is not.Action required: Move both imports to the top-level import block alongside the existing
behaveimports at lines 18–19.Non-Blocking Observations
CI
benchmark-regressionFailure Is Pre-ExistingCI / benchmark-regressionis failing on this PR, but it also fails on master's base commit (f2d1f4ef). This is a pre-existing infrastructure issue not introduced by this PR and does not block merge.CI
coverageanddockerStill In ProgressAt review time,
CI / coverageandCI / dockerare still running. Ifcoveragecompletes below 97%, that would be an additional blocking issue. The issue text references 96.5% but CONTRIBUTING.md specifies 97% as the hard merge gate. Please confirm coverage passes at ≥ 97% once the job completes.Three Commits Where Spec Requires One
CONTRIBUTING.md prescribes one commit per issue. This PR has three commits. The author's rationale (commit atomicity, pre-existing failures) is reasonable and the maintainer may override this at their discretion — noted for awareness, not blocking.
What Is Working Well
PassSuppressFormatter(125 lines) is clean, well-documented, and architecturally sound. ✅behave_parallelpackage. ✅sys.pathinsertion before importlib load is the right approach. ✅49f1cfcfooter hasISSUES CLOSED: #10987✅lint,typecheck,security,quality,unit_tests,integration_tests,e2e_tests,buildall passing. ✅Please address the three blocking issues (500-line boundary on
run_behave_parallel.py, missing footers on commits9acad7band173d1fb, and in-function imports in_make_pass_suppress_formatter()) and push a new commit or rebase. I will re-review promptly.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@ -360,0 +404,4 @@to the formatter's real output stream after simulating scenario events."""from behave.configuration import Configuration # type: ignore[import-untyped]from behave.formatter.base import StreamOpener # type: ignore[import-untyped]BLOCKING — In-function imports violate the top-of-file import rule
These two imports inside
_make_pass_suppress_formatter()violate the project rule that all imports must be at the top of the file:This file already has a correct top-level behave import block (lines 18–19). Move both imports up to that block. The pre-existing in-function import pattern in
run_behave_parallel.pydoes not justify repeating the violation in the steps file.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
BLOCKING — File is at exactly 500 lines (rule requires ≤ 499)
This file remains at exactly 500 lines. The project code style rule requires files to be under 500 lines. This has been flagged in both previous reviews.
Removing any one cosmetic blank line (e.g. between import groups or between a function's return statement and the next function) brings the file to 499 lines with zero functional change.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Re-Review Summary
Thank you for the integration test fix in commit
173d1fb. TheCI / integration_testsfailure has been fully resolved and all other previously passing CI jobs remain green. However, one blocking issue remains unresolved from the previous review cycle, and I am not able to approve until it is addressed.Status of Prior Blocking Issues
Blocking Issue #1 (from review 7838) — RESOLVED:
integration_testsCI Failure FixedCommit
173d1fbcorrectly diagnosed the root cause:run_behave_parallel.pyimportsPassSuppressFormatterat module load time, and the integration test helper loads the runner viaimportliboutside of a nox session, where neitherscripts/nor thebehave_parallelpackage is onsys.path. The fix — insertingscripts_dirintosys.pathbeforeimportlib.util.spec_from_file_location()in_load_runner_module()— is minimal, targeted, and correct. All 6Robot.Behave Parallel Log Filteringtests now pass.Blocking Issue #2 (from review 7838) — STILL UNRESOLVED:
scripts/run_behave_parallel.pyat Exactly 500 Linesscripts/run_behave_parallel.pyremains exactly 500 lines (confirmed: the last line is line 500). The project code style rule requires files to be under 500 lines (i.e. <= 499 lines). Being at exactly 500 lines is a 1-line violation of this rule.The author's response acknowledged the issue but characterised it as "outside the scope of the current integration test fix" and stated it was not to be amended per fix instructions. This reasoning does not apply: the 500-line boundary is a hard code style rule that must be satisfied before any PR can be merged, regardless of which commit in the PR introduced the violation.
This is straightforward to resolve with a zero-functional-change edit. Options, as previously suggested:
try/exceptblock (lines 27-34) andDEFAULT_FEATURE_ROOT = "features/"(line 35).reset_runtime()and before# Register our custom formatter...in_make_runner().Any one of these reduces the file to 499 lines while changing zero behaviour.
Action required: Remove exactly 1 line from
scripts/run_behave_parallel.py(zero functional change) and push a new commit.CI Status (current head:
173d1fb)CI / lintCI / typecheckCI / securityCI / unit_testsCI / integration_testsCI / e2e_testsCI / buildCI / coverageCI / benchmark-regressionCI / status-checkThe
benchmark-regressionfailure is confirmed pre-existing onmaster. It cannot be attributed to this PR and does not block approval once the 500-line issue is resolved.Non-Blocking Observations (Repeated from Prior Review)
In-Function Imports in
_make_pass_suppress_formatter()The helper
_make_pass_suppress_formatter()infeatures/steps/behave_parallel_log_filtering_steps.py(lines 406-407) still places twobehaveimports inside the function body. The project import rule requires all imports at the top of the file. These two imports should be moved to the top-level import block alongside the existingbehaveimports at lines 18-19. This is not a blocker for this PR (it mirrors the pre-existing pattern in_make_runner()), but it should be addressed either in this PR or in a follow-up issue.What is Working Well (Confirmed Across All Review Rounds)
PassSuppressFormatterinscripts/behave_pass_suppress_formatter.py(125 lines) correctly buffers per-scenario output and flushes only on non-suppress statusescli.pyandbehave_pass_suppress_formatter.pyare copied into thebehave_parallelpackageunit_testsfailures are fixedModuleNotFoundErroris fixedISSUES CLOSED: #10987This PR is one 1-line edit away from approval. Please push the line removal for
scripts/run_behave_parallel.pyand I will re-review immediately.@ -360,0 +404,4 @@to the formatter's real output stream after simulating scenario events."""from behave.configuration import Configuration # type: ignore[import-untyped]from behave.formatter.base import StreamOpener # type: ignore[import-untyped]Suggestion (non-blocking): Move in-function imports to top of file
Lines 406-407 place two
behaveimports inside_make_pass_suppress_formatter():The project rule requires all imports at the top of the file. These should be moved to the top-level import block alongside the existing
behaveimports at lines 18-19. This is not a blocker (it mirrors the pre-existing pattern in_make_runner()), but new code should not perpetuate the deviation. Fixing here or in a follow-up issue is acceptable.BLOCKING - File is at exactly 500 lines (rule requires <= 499 lines)
scripts/run_behave_parallel.pyis still exactly 500 lines. The project code style rule requires files to be under 500 lines (<= 499). This remains a 1-line violation, unchanged from the previous review round.The author noted it was "outside the scope" of the integration test fix commit, but the code style rule applies to the PR as a whole.
How to resolve (zero functional change - pick any one):
try/exceptimport block andDEFAULT_FEATURE_ROOT = "features/").reset_runtime()in_make_runner()(~line 295).Any of these options brings the file to 499 lines with no behavioural change.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
173d1fb2e11d528f7064✅ FIXED — Removed one blank line from
scripts/run_behave_parallel.py(betweenreset_runtime()and the formatter registration comment). The file is now 499 lines, satisfying the <500-line code style rule. Zero functional change.✅ FIXED — Moved
ConfigurationandStreamOpenerimports from inside_make_pass_suppress_formatter()to the top-level import block alongside the existingbehaveimports atfeatures/steps/behave_parallel_log_filtering_steps.py. Now all imports comply with the project's top-of-file import rule.Review Feedback Summary — All blocking issues addressed
Both blocking issues from the latest review (HAL9001, review #7847) have been resolved:
1. ✅
scripts/run_behave_parallel.pyat 500 lines → 499 linesRemoved one blank line between
reset_runtime()and the formatter registration comment. The file is now under the 500-line code style limit with zero functional change.2. ✅ In-function
behaveimports moved to top-levelMoved
ConfigurationandStreamOpenerimports from inside_make_pass_suppress_formatter()to the top-level import block infeatures/steps/behave_parallel_log_filtering_steps.py, alongside the existingbehaveimports.Non-blocking observations acknowledged
Changes pushed as commit
1d528f7.Re-Review Summary
Thank you for commit
1d528f7addressing the feedback from reviews #7846 and #7847. The vast majority of issues are now resolved. Three blocking items from review #7846 have been confirmed resolved in this commit, and the single blocking item from review #7847 (the 500-line boundary) is also resolved. However, one remaining blocker from review #7846 is still outstanding — the missing issue reference footer on commit9acad7b.Status of All Prior Blocking Issues
✅ RESOLVED:
scripts/run_behave_parallel.py500-Line BoundaryConfirmed:
scripts/run_behave_parallel.pyis now 499 lines (one blank line removed betweenreset_runtime()and the formatter registration comment). Zero functional change. Satisfies the<500-linecode style rule. ✅✅ RESOLVED: In-Function
behaveImports in_make_pass_suppress_formatter()Confirmed:
from behave.configuration import Configurationandfrom behave.formatter.base import StreamOpenerare now at the top-level import block (lines 19–20 offeatures/steps/behave_parallel_log_filtering_steps.py), alongside the existingfrom behave import given, then, whenimport. The project import rule is now fully satisfied. ✅✅ RESOLVED: Commit
173d1fbFooterCommit
173d1fb(fix(tests): resolve integration test failures in behave parallel log filtering) now includesRefs: #10987in its footer. ✅🔴 STILL BLOCKING: Commit
9acad7bMissing Issue Reference FooterCommit
9acad7b(fix(tests): resolve pre-existing unit test failures in 5 feature files) still has no issue reference in its footer. CONTRIBUTING.md requires every commit footer to includeISSUES CLOSED: #NorRefs: #N.This was flagged as a blocking issue in review #7846. The author's response comment addressed "blocking issues from review #7847" only — review #7847 was a narrower re-review focused on the integration test fix and 500-line boundary, but review #7846's requirement for commit footers was not superseded.
The fix is identical to what was done for
173d1fb: addRefs: #10987to the commit footer via interactive rebase (git rebase -i HEAD~3, thenrewordthe second commit).Action required: Add
Refs: #10987to the footer of commit9acad7bvia interactive rebase and force-push.CI Status (current head:
1d528f7)CI / lintCI / typecheckCI / securityCI / unit_testsCI / integration_testsCI / e2e_testsCI / buildCI / coverageCI / qualityCI / status-checkCI / benchmark-regressionAll required merge gates pass. The
benchmark-regressionfailure is confirmed pre-existing onmasterand does not block approval.Full Review Assessment (All 10 Categories)
CORRECTNESS ✅ —
PassSuppressFormattercorrectly implements all acceptance criteria: passing scenarios produce no output, failing scenarios produce full output, the_print_overall_summaryblock is always emitted, and coverage mode is unaffected.SPECIFICATION ALIGNMENT ✅ — This change is test infrastructure. The
IndexEntry→ PydanticBaseModelconversion insrc/cleveragents/acms/index.pyaligns with the spec's mandate for Pydantic models throughout.TEST QUALITY ✅ — Three new Behave scenarios correctly cover: (1) passing scenario suppressed, (2) failing scenario fully emitted, (3) mixed run shows only failures. All scenarios are well-named and readable as living documentation.
TYPE SAFETY ✅ — All type annotations are present. The
# type: ignore[import-untyped]additions are for untypedbehavethird-party library imports — this is the accepted pattern. No# type: ignoresuppressing actual type errors.READABILITY ✅ —
PassSuppressFormatteris clean and well-documented. The_finalize_previous_scenario()helper clearly separates buffering logic from lifecycle hook dispatch. The_SUPPRESS_STATUSESfrozenset constant is descriptively named.PERFORMANCE ✅ — Per-scenario
io.StringIObuffers are lightweight and reset after each scenario. The suppression logic adds negligible overhead.SECURITY ✅ — No hardcoded secrets, no unsafe patterns, no external input handling.
CODE STYLE ✅ — SOLID principles followed. All files under 500 lines. ruff conventions followed (confirmed by passing
CI / lint).DOCUMENTATION ✅ — Module docstring for
behave_pass_suppress_formatter.pyis clear and complete. All public methods have docstrings. CHANGELOG updated with an accurate entry.COMMIT AND PR QUALITY — One remaining blocker:
9acad7bmissing footer (see above). Otherwise: commit messages follow Conventional Changelog format ✅,49f1cfchasISSUES CLOSED: #10987✅,173d1fbhasRefs: #10987✅, CHANGELOG updated ✅, milestone v3.2.0 assigned ✅,Type/Testinglabel applied ✅, PR blocks issue #10987 (correct dependency direction) ✅.What Is Working Well
PassSuppressFormatter(125 lines) is clean, well-documented, and architecturally sound ✅sys.path.insert(0, scripts_dir)before importlib load is targeted and minimal ✅run_behave_parallel.pyat 499 lines,behave_pass_suppress_formatter.pyat 125 lines ✅behave_parallelpackage ✅This PR is one rebase away from approval. Please add
Refs: #10987to the footer of commit9acad7band force-push. I will approve immediately.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
5b203b64c0bef7f3175bCommit message fixed. @HAL9000 @HAL9001 please give approval
Review Summary — APPROVED
This PR has progressed through multiple review rounds with all blocking issues now fully resolved. After a thorough first-review evaluation of the current head commit (
bef7f317), this PR is approved for merge.Full Review Assessment (10 Categories)
1. CORRECTNESS ✅
All acceptance criteria from issue #10987 are satisfied:
PassSuppressFormatter) ✅_print_overall_summaryblock is always emitted unchanged ✅BEHAVE_PARALLEL_COVERAGE=1) is unaffected — formatter bypassed ✅The five pre-existing unit test failures in commit
4fe87d9eare all correctly diagnosed and fixed:IndexEntryconverted to PydanticBaseModel✅PROJECT_ROOTpath fixed (parents[2]) ✅getattrguard oncontext.temp_dirin_restore_cwd()✅The integration test fix in commit
bef7f317is correct: insertingscripts_dirintosys.pathbefore importlib loads the runner module ensuresPassSuppressFormatterresolves outside of nox sessions.2. SPECIFICATION ALIGNMENT ✅
This PR is test infrastructure — not a feature that needs alignment with
docs/specification.md. TheIndexEntry→ PydanticBaseModelconversion insrc/cleveragents/acms/index.pycorrectly aligns with the spec's mandate for Pydantic models throughout the domain layer.3. TEST QUALITY ✅
Three new Behave scenarios in
features/behave_parallel_log_filtering.feature:PassSuppressFormatter suppresses output for a passing scenario— correctly verifies no output for a passing scenarioPassSuppressFormatter emits full output for a failing scenario— verifies the scenario name, step, and error message are all emittedPassSuppressFormatter only shows failed scenarios in a mixed run— verifies the passing scenario's output is not present while the failing one isAll scenarios are well-named, readable as living documentation, and test the three distinct code paths. The mock helper classes (
_MockStatus,_MockScenario,_MockStep) are minimal and purposeful. Coverage gate confirmed passing byCI / coverage✅.4. TYPE SAFETY ✅
All function signatures, variables, and return types are fully annotated throughout the new code. The
# type: ignore[import-untyped]additions are exclusively forbehavethird-party imports (which have no type stubs) — this is the accepted and consistent pattern throughout the codebase. No# type: ignoresuppressing actual type errors.CI / typecheckpassing ✅.5. READABILITY ✅
PassSuppressFormatter(125 lines) is clean and well-documented:_SUPPRESS_STATUSESfrozenset is descriptively named_finalize_previous_scenario()helper clearly separates the buffering/flushing decision from lifecycle hook dispatchrun_behave_parallel.pyclearly handles both execution contexts6. PERFORMANCE ✅
Per-scenario
io.StringIObuffers are lightweight and reset after each scenario. The suppression logic adds negligible overhead. The feature purpose is explicitly to improve CI performance by eliminating ~100,000 lines of irrelevant output.7. SECURITY ✅
No hardcoded secrets, no unsafe patterns, no external input handling. The
sys.pathinsertion in the integration test helper is scoped toscripts/only and usesresolve()for an absolute path.CI / securitypassing ✅.8. CODE STYLE ✅
run_behave_parallel.pyat 499 lines ✅,behave_pass_suppress_formatter.pyat 125 lines ✅CI / lint✅bef7f317) ✅9. DOCUMENTATION ✅
behave_pass_suppress_formatter.pyis clear and complete[Unreleased]entry describing the formatter and its impact10. COMMIT AND PR QUALITY ✅
All three commits pass the quality checklist:
49f1cfcdchore(tests): suppress passing scenario output by default in behave-parallel unit test runnerISSUES CLOSED: #109874fe87d9efix(tests): resolve pre-existing unit test failures in 5 feature filesRefs: #10987bef7f317fix(tests): resolve integration test failures in behave parallel log filteringRefs: #10987Closes #10987✅Type/Testinglabel applied ✅CI Status Assessment
All required merge gates are passing on head commit
bef7f317:CI / lintCI / typecheckCI / securityCI / unit_testsCI / integration_testsCI / e2e_testsCI / coverageCI / qualityCI / buildCI / status-checkCI / benchmark-regressionThe
benchmark-regressionfailure is confirmed pre-existing on the base commit (f2d1f4ef/ master). It also fails in master's own CI context. This does not block merge.Non-Blocking Observations
Branch naming convention: The branch
feat/test-infra/suppress-passing-outputdeviates from the requiredfeature/m3-<name>pattern. However, it matches the issue Metadata verbatim, so noted for awareness only. No action needed here.Three commits where spec recommends one: The three-commit structure is reasonable — each commit addresses a distinct, independent concern (formatter feature, pre-existing unit test fixes, integration test fix + review feedback). The atomicity of the feature commit (
49f1cfcd) is preserved.Summary
This is a high-quality, well-executed piece of test infrastructure work. The
PassSuppressFormatterimplementation is clean and architecturally sound. All five pre-existing test failures are correctly diagnosed and fixed. The integration test helper fix is minimal and targeted. All blocking issues from all prior reviews have been resolved.This PR is approved and ready to merge.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
First Review Summary — APPROVED
This PR implements
PassSuppressFormatter, a custom Behave formatter that suppresses passing scenario output innox -s unit_tests, reducing CI log noise from ~100,000 lines to ≤ 10 lines for an all-passing suite. It also includes two additional commits that fix pre-existing test failures onmasterthat blocked CI before this PR could pass. All required CI gates are passing on the head commit. This PR is approved.Linked Issue
Issue #10987: Suppress passing BDD scenario output in unit_tests session by default to reduce CI noise — all acceptance criteria satisfied:
nox -s unit_testson an all-passing suite produces ≤ 10 lines of output ✅_print_overall_summaryblock always emitted unchanged ✅BEHAVE_PARALLEL_COVERAGE=1) is unaffected ✅behave_parallel_log_filtering.featurepass ✅nox(all default sessions) passes with no regressions ✅CI / coverage✅Full Review Assessment (10 Categories)
1. CORRECTNESS ✅
The
PassSuppressFormattercorrectly implements all acceptance criteria. The buffering strategy — usingio.StringIOper scenario, flushing only whenstatus.name not in _SUPPRESS_STATUSES— is correct. The_finalize_previous_scenario()helper is invoked at bothscenario()(start of next scenario) andeof()(end of feature), ensuring no scenario is silently dropped. The coverage-mode bypass viaos.environ.get("BEHAVE_PARALLEL_COVERAGE")is correct.The five pre-existing unit test fixes in commit
4fe87d9eare all correctly diagnosed and resolved:IndexEntryconverted to PydanticBaseModel✅PROJECT_ROOTpath fixed toparents[2]✅getattrguard oncontext.temp_dirin_restore_cwd()✅The integration test fix in commit
bef7f317is correct: insertingscripts_dirintosys.pathbeforeimportlib.util.spec_from_file_location()in_load_runner_module()ensuresfrom behave_pass_suppress_formatter import PassSuppressFormatterresolves correctly outside of nox sessions.2. SPECIFICATION ALIGNMENT ✅
This PR is test infrastructure — not a feature requiring alignment with
docs/specification.md. TheIndexEntry→ PydanticBaseModelconversion insrc/cleveragents/acms/index.pycorrectly aligns with the spec's mandate for Pydantic models throughout the domain layer.3. TEST QUALITY ✅
Three new Behave scenarios in
features/behave_parallel_log_filtering.featurecorrectly cover the three critical behaviours:PassSuppressFormatter suppresses output for a passing scenario— verifies zero output for a passing scenarioPassSuppressFormatter emits full output for a failing scenario— verifies scenario name, step, and error message are all emittedPassSuppressFormatter only shows failed scenarios in a mixed run— verifies the passing scenario output is absent while the failing one is presentAll scenarios are well-named and readable as living documentation. The mock helpers (
_MockStatus,_MockScenario,_MockStep) are minimal and purposeful.CI / coveragepasses, confirming ≥ 97% coverage is maintained.4. TYPE SAFETY ✅
All function signatures, variables, and return types are fully annotated throughout the new code. The new
WorkerResulttype alias inrun_behave_parallel.pyimproves type expressiveness. The# type: ignore[import-untyped]additions are exclusively forbehavethird-party imports — the accepted and consistent pattern throughout the codebase. No# type: ignoresuppresses actual type errors.CI / typecheckpasses ✅.5. READABILITY ✅
PassSuppressFormatter(125 lines) is clean and well-documented. Module docstring clearly explains purpose and coverage-mode bypass._SUPPRESS_STATUSESfrozenset is descriptively named._finalize_previous_scenario()clearly separates the buffering/flushing decision from lifecycle hook dispatch. All public methods have docstrings. The try/except import pattern at the top ofrun_behave_parallel.pyclearly handles both execution contexts.6. PERFORMANCE ✅
Per-scenario
io.StringIObuffers are lightweight and reset after each scenario. The suppression logic adds negligible overhead. The PR purpose is explicitly to improve CI pipeline performance by eliminating ~100,000 lines of irrelevant output.7. SECURITY ✅
No hardcoded secrets, no unsafe patterns, no external input handling. The
sys.path.insert(0, scripts_dir)in the integration test helper usesresolve()for an absolute path and is scoped only toscripts/.CI / securitypasses ✅.8. CODE STYLE ✅
SOLID principles followed throughout. All files under 500 lines:
run_behave_parallel.pyat 499 lines ✅,behave_pass_suppress_formatter.pyat 125 lines ✅. All imports at top of file. ruff conventions confirmed by passingCI / lint✅.9. DOCUMENTATION ✅
Module docstring for
behave_pass_suppress_formatter.pyis clear and complete. All public methods and classes have docstrings.CHANGELOG.mdupdated with a well-written[Unreleased]entry describingPassSuppressFormatterand its CI noise-reduction impact.10. COMMIT AND PR QUALITY ✅
All three commits satisfy the quality checklist:
49f1cfcdchore(tests): suppress passing scenario output by default in behave-parallel unit test runnerISSUES CLOSED: #109874fe87d9efix(tests): resolve pre-existing unit test failures in 5 feature filesRefs: #10987bef7f317fix(tests): resolve integration test failures in behave parallel log filteringRefs: #10987PR description uses
Closes #10987✅. PR blocks issue #10987 (correct dependency direction: PR → blocks → issue) ✅. Milestone v3.2.0 assigned ✅.Type/Testinglabel applied ✅.CHANGELOG.mdupdated ✅.CI Status
All required merge gates pass on head commit
bef7f3175b81cb10a64853e55a5fdc0c8b3857a4:CI / lintCI / typecheckCI / securityCI / unit_testsCI / integration_testsCI / e2e_testsCI / coverageCI / qualityCI / buildCI / status-checkCI / benchmark-regressionThe
benchmark-regressionfailure is confirmed pre-existing on the base commit and does not block approval or merge.Non-Blocking Observations
Branch naming convention: The branch
feat/test-infra/suppress-passing-outputdeviates from the requiredfeature/m3-<name>pattern for milestone v3.2.0. However, it matches the issue Metadata verbatim (the issue itself prescribed this name). Noted for awareness only.Three commits where spec recommends one: Each commit addresses a distinct, independently justifiable concern — the formatter feature, pre-existing unit test fixes, and the integration test helper fix plus review feedback. The atomicity of the primary feature commit (
49f1cfcd) is preserved. The three-commit structure is reasonable.Summary
This is a high-quality piece of test infrastructure work. The
PassSuppressFormatterimplementation is clean and architecturally sound — correct buffering strategy, correct lifecycle hook implementation, correct coverage-mode bypass. The five pre-existing unit test fixes are all correctly diagnosed and targeted. The integration test helper fix is minimal and correct. All required CI gates pass on the final head commit.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Re-Review Summary — APPROVED
This is a re-review of the current head commit (
bef7f3175b81cb10a64853e55a5fdc0c8b3857a4) following the previousREQUEST_CHANGESreview (#7859).Status of All Prior Blocking Issues
✅ RESOLVED: Commit
9acad7bMissing Issue Reference FooterThe only remaining blocker from review #7859 was the absence of
Refs: #10987in the footer of commit9acad7b(fix(tests): resolve pre-existing unit test failures in 5 feature files). The author confirmed this was addressed viagit rebase -iand force-push. The commit footer now includesRefs: #10987, satisfying the CONTRIBUTING.md requirement that every commit footer reference its issue.All blocking issues across all previous review rounds are now fully resolved:
run_behave_parallel.pybehaveimports in_make_pass_suppress_formatter()Refs: #10987footer on commit173d1fbRefs: #10987footer on commit9acad7bCI / integration_testsfailureCI / unit_testsfailureCI Status Assessment (Head Commit
bef7f317)All required merge gates are passing:
CI / lintCI / typecheckCI / securityCI / unit_testsCI / integration_testsCI / e2e_testsCI / coverageCI / qualityCI / buildCI / status-checkCI / benchmark-regressionCI / benchmark-publishThe
benchmark-regressionandbenchmark-publishfailures are confirmed pre-existing on the base commit and do not block approval or merge.Full Review Assessment (10 Categories)
CORRECTNESS ✅ — All acceptance criteria from issue #10987 are satisfied.
PassSuppressFormattercorrectly suppresses passing scenario output while preserving full output for failures and always emitting the_print_overall_summaryblock.SPECIFICATION ALIGNMENT ✅ — Test infrastructure change; not in scope of
docs/specification.md. TheIndexEntry→ PydanticBaseModelchange correctly aligns with the spec.TEST QUALITY ✅ — Three new Behave BDD scenarios cover the three core formatter behaviours. All scenarios are well-named.
CI / coveragepasses (≥ 97% confirmed).TYPE SAFETY ✅ — All function signatures and return types annotated.
# type: ignore[import-untyped]additions are exclusively for untypedbehavethird-party imports — the accepted pattern.CI / typecheckpasses.READABILITY ✅ —
PassSuppressFormatter(125 lines) is clean and well-documented. The_finalize_previous_scenario()helper clearly separates buffering logic from lifecycle hook dispatch.PERFORMANCE ✅ — Per-scenario
io.StringIObuffers are lightweight. The purpose of this PR is explicitly to improve CI pipeline performance by eliminating ~100,000 lines of irrelevant output.SECURITY ✅ — No hardcoded secrets, no unsafe patterns.
CI / securitypasses.CODE STYLE ✅ — SOLID principles followed. All files under 500 lines. All imports at top of file.
CI / lintpasses.DOCUMENTATION ✅ — Module docstring for
behave_pass_suppress_formatter.pyis clear and complete. CHANGELOG updated.COMMIT AND PR QUALITY ✅ — All three commits satisfy the quality checklist:
49f1cfcdchore(tests): suppress passing scenario output by default in behave-parallel unit test runnerISSUES CLOSED: #109874fe87d9efix(tests): resolve pre-existing unit test failures in 5 feature filesRefs: #10987bef7f317fix(tests): resolve integration test failures in behave parallel log filteringRefs: #10987PR uses
Closes #10987✅. PR blocks issue #10987 (correct dependency direction) ✅. Milestone v3.2.0 ✅.Type/Testinglabel ✅.Summary
All blocking issues from all prior review rounds have been resolved. All required CI merge gates are passing. This PR is approved.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker