fix(test): replace vacuous assertions with real verification in BDD step definitions #728

Open
opened 2026-03-12 13:18:03 +00:00 by CoreRasurae · 0 comments
Member

Metadata

  • Commit Message: fix(test): replace vacuous assertions with real verification in BDD step definitions
  • Branch: fix/m3-vacuous-step-assertions

Summary

Six @then step definitions in features/steps/cli_plan_context_commands_steps.py contain assertions that either silently pass when preconditions are false, have tautological assertions, or have pass as their body. This means tests using these steps can pass without actually verifying the expected behavior.

Background and Context

Identified during the code review of PR #727 (Finding #5 in the APPROVED review by @hurui200320). These are pre-existing issues not introduced by that PR.

Current Behavior

The following @then step definitions do not perform meaningful verification:

  1. Lines 464-470 (step_plan_has_changes): Passes silently if the changes file doesn't exist — the step succeeds even when the plan has no changes file at all.
  2. Lines 473-484 (step_changes_in_database): The else branch contains a tautological assertion that always passes.
  3. Lines 493-499 (step_changes_marked_applied): Passes silently if the file doesn't exist — same issue as #1.
  4. Lines 590-595 (step_equivalent_command): Body is pass — the step does nothing.
  5. Lines 414-421 (step_project_named): Body is pass — the step does nothing.
  6. Lines 302-307 (step_plan_has_conversation): Body is pass — the step does nothing.

Expected Behavior

Every @then step definition should either:

  • Contain a meaningful assertion that fails when the expected condition is not met, OR
  • Be explicitly marked as pending (using Behave's @pending decorator or context.scenario.skip()) so it is visible in test reports as unimplemented.

Acceptance Criteria

  • All 6 identified step definitions contain real assertions or are marked as pending
  • Steps that check file existence fail explicitly (via assert or raise) when the file is missing, rather than silently passing
  • No tautological assertions remain
  • All existing scenarios that use these steps still pass
  • No new test failures introduced

Supporting Information

Subtasks

  • Fix step_plan_has_changes (lines 464-470): Assert file exists before checking contents
  • Fix step_changes_in_database (lines 473-484): Replace tautological else-branch assertion
  • Fix step_changes_marked_applied (lines 493-499): Assert file exists before checking contents
  • Fix step_equivalent_command (lines 590-595): Implement real assertion or mark pending
  • Fix step_project_named (lines 414-421): Implement real assertion or mark pending
  • Fix step_plan_has_conversation (lines 302-307): Implement real assertion or mark pending
  • Run nox -s unit_tests to verify no regressions

Definition of Done

This issue is complete when:

  • All subtasks above are completed and checked off.
  • A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
  • The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
  • The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
## Metadata - **Commit Message**: `fix(test): replace vacuous assertions with real verification in BDD step definitions` - **Branch**: `fix/m3-vacuous-step-assertions` ## Summary Six `@then` step definitions in `features/steps/cli_plan_context_commands_steps.py` contain assertions that either silently pass when preconditions are false, have tautological assertions, or have `pass` as their body. This means tests using these steps can pass without actually verifying the expected behavior. ## Background and Context Identified during the code review of PR #727 (Finding #5 in the APPROVED review by @hurui200320). These are pre-existing issues not introduced by that PR. ## Current Behavior The following `@then` step definitions do not perform meaningful verification: 1. **Lines 464-470** (`step_plan_has_changes`): Passes silently if the changes file doesn't exist — the step succeeds even when the plan has no changes file at all. 2. **Lines 473-484** (`step_changes_in_database`): The `else` branch contains a tautological assertion that always passes. 3. **Lines 493-499** (`step_changes_marked_applied`): Passes silently if the file doesn't exist — same issue as #1. 4. **Lines 590-595** (`step_equivalent_command`): Body is `pass` — the step does nothing. 5. **Lines 414-421** (`step_project_named`): Body is `pass` — the step does nothing. 6. **Lines 302-307** (`step_plan_has_conversation`): Body is `pass` — the step does nothing. ## Expected Behavior Every `@then` step definition should either: - Contain a meaningful assertion that fails when the expected condition is not met, OR - Be explicitly marked as `pending` (using Behave's `@pending` decorator or `context.scenario.skip()`) so it is visible in test reports as unimplemented. ## Acceptance Criteria - [ ] All 6 identified step definitions contain real assertions or are marked as pending - [ ] Steps that check file existence fail explicitly (via `assert` or `raise`) when the file is missing, rather than silently passing - [ ] No tautological assertions remain - [ ] All existing scenarios that use these steps still pass - [ ] No new test failures introduced ## Supporting Information - Identified in: PR #727 Code Review, Finding #5 (APPROVED review) - Review URL: https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/727 - Reviewer: @hurui200320 ## Subtasks - [ ] Fix `step_plan_has_changes` (lines 464-470): Assert file exists before checking contents - [ ] Fix `step_changes_in_database` (lines 473-484): Replace tautological else-branch assertion - [ ] Fix `step_changes_marked_applied` (lines 493-499): Assert file exists before checking contents - [ ] Fix `step_equivalent_command` (lines 590-595): Implement real assertion or mark pending - [ ] Fix `step_project_named` (lines 414-421): Implement real assertion or mark pending - [ ] Fix `step_plan_has_conversation` (lines 302-307): Implement real assertion or mark pending - [ ] Run `nox -s unit_tests` to verify no regressions ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done.
freemo added this to the v3.2.0 milestone 2026-03-12 21:01:12 +00:00
freemo self-assigned this 2026-04-02 06:13:51 +00:00
Sign in to join this conversation.
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.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core#728
No description provided.