Files
cleveragents-core/features/tdd_server_connect_atomic_writes.feature
hurui200320 2005b8ef82
CI / push-validation (push) Successful in 18s
CI / build (push) Successful in 19s
CI / helm (push) Successful in 24s
CI / lint (push) Successful in 29s
CI / security (push) Successful in 1m11s
CI / e2e_tests (push) Successful in 2m56s
CI / quality (push) Successful in 3m40s
CI / typecheck (push) Successful in 3m59s
CI / integration_tests (push) Successful in 4m3s
CI / unit_tests (push) Successful in 4m55s
CI / docker (push) Successful in 10s
CI / coverage (push) Successful in 10m44s
CI / status-check (push) Successful in 1s
CI / benchmark-regression (push) Has been skipped
CI / benchmark-publish (push) Successful in 1h13m28s
feat(tests): replace all @skip tags with proper @tdd_expected_fail tags or remove them across the entire codebase (#7221)
## Summary

Replaces all 234 bare `@skip` occurrences across 82 Behave feature files with the correct TDD issue-capture tagging system described in CONTRIBUTING.md § Bug Fix Workflow.

Previously, the noxfile ran Behave with `--tags=not @skip`, silently excluding all `@skip`-tagged scenarios from every CI run. These tests never ran, never inverted results via the `@tdd_expected_fail` mechanism, and never contributed to coverage — defeating the purpose of TDD issue-capture testing. Every `@skip` occurrence had a commented-out hint line immediately above it showing the intended proper tags (e.g., `# @tdd_issue @tdd_issue_4272 @tdd_expected_fail @skip`), confirming they were all intended for conversion.

## Changes

### Mechanical conversion (234 replacements across 82 files)
- Extracted the proper TDD tags from the comment hint above each `@skip` line, removed `@skip` from the tag set, and replaced the `@skip` line with those tags.
- Removed the now-redundant comment hint lines alongside each replacement.

### Bug-fixed scenarios — `@tdd_expected_fail` removed (84 scenarios)
- After conversion, ran `nox -s unit_tests` to identify which newly-enabled `@tdd_expected_fail` scenarios now **pass** (their referenced bugs have already been fixed). Removed `@tdd_expected_fail` from those 84 scenarios and their corresponding feature-level tags, leaving only the permanent `@tdd_issue @tdd_issue_<N>` regression-guard tags.
- Affected features include: `tdd_tool_runner_env_precedence`, `tdd_automation_profile_session_leak`, `tls_certificate_check`, `project_create_persist`, `resource_type_bootstrap_*`, and 18 others.

### Noxfile cleanup
- Removed all four `--tags=not @skip` arguments from `noxfile.py` (unit_tests and coverage sessions). With zero `@skip` tags remaining in the codebase, this filter was dead code and its presence would mislead future maintainers into thinking `@skip` is still a supported escape mechanism.

### Regression guard files
- Split the regression guards into two focused files:
  - `tdd_regression_guards_exec_env.feature` for bug #4281 (exec-env precedence)
  - `tdd_regression_guards_session_list.feature` for bug #4271 (session list summary)
- Each file carries only its own `@tdd_issue` tags at the feature level, avoiding cross-contamination via Behave tag inheritance. The `Background` step (`session-list-summary mock`) only appears in the session-list file where it is actually needed.

### Duplicate tag cleanup
- Removed duplicate `@tdd_issue @tdd_issue_4287` tag lines in `tdd_skill_add_regression.feature` (lines 20 and 29).

### Inline comment for retained `@tdd_expected_fail`
- Added inline comment in `ci_workflow_validation.feature:134` explaining why this specific #4227 scenario retains `@tdd_expected_fail` despite #4227 being closed (CI YAML does not encode threshold as a machine-readable value).

### Known edge cases — `@tdd_expected_fail` retained (closed issues, fix on master, scenarios still fail)
The following issues are **closed** and their fixes **are on master**, but the specific test assertions still fail because the fixes address other aspects of the bugs. The `@tdd_expected_fail` tags are functionally correct and must remain until the specific scenario assertions pass:
- `tdd_exec_env_resolution_precedence.feature` — bug #1080 (closed 2026-03-31). The precedence-level-2-vs-4 scenario still fails.
- `session_list_summary_dedup.feature` — bug #3046 (closed 2026-04-05). The dedup-consistency scenarios still fail.
- `actor_add_update_enforcement.feature` — bug #2609 (closed 2026-04-05). The enforcement scenarios still fail.
- `ci_workflow_validation.feature:134` — #4227 (closed 2026-04-08). The CI YAML threshold assertion still fails.

## Verification

- `grep -r "@skip" features/ --include="*.feature"` → **zero results** ✓
- `grep -n "tags=not @skip" noxfile.py` → **zero results** ✓
- `nox -s unit_tests` → **629 features passed, 0 failed** ✓ (up from ~545 before this PR)
- CI all green (coverage ≥ 97%) ✓
- Integration tests (Robot Framework) do not use `@skip` — confirmed no action needed ✓
- E2E tests (Robot Framework) do not use `@skip` — confirmed no action needed ✓
- `CHANGELOG.md` updated with entry for this change ✓
- `CONTRIBUTORS.md` — Rui Hu already listed ✓

## Issues Addressed

Closes #7025

Co-authored-by: CleverThis <hal9000@cleverthis.com>
Reviewed-on: #7221
Reviewed-by: HAL 9000 <HAL9000@cleverthis.com>
Reviewed-by: HAL9001 <hal9001@cleverthis.com>
Co-authored-by: Rui Hu <rui.hu@cleverthis.com>
Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
2026-04-13 04:56:01 +00:00

45 lines
2.5 KiB
Gherkin

@tdd_expected_fail @tdd_issue @tdd_issue_993
Feature: TDD Issue #993 — server_connect writes three config values non-atomically
As a developer
I want to verify that server_connect writes all three config values atomically
So that a partial failure does not leave the configuration in a half-written state
# This test captures bug #993: server_connect in server.py makes three
# sequential set_value() calls (server.url, server.namespace, server.tls-verify)
# with no try/except, no transaction, and no rollback. If the second call
# fails (e.g. disk full, permissions error), server.url is already persisted
# but server.namespace and server.tls-verify retain their old values. The
# config is left in a half-written state.
#
# Expected behavior: all three config values are written atomically either
# all succeed or all fail (rollback to original state).
#
# This test uses the @tdd_expected_fail tag until the fix in #993 is merged.
# The tag inverts the result so CI passes while the bug is still unfixed.
# See CONTRIBUTING.md > Bug Fix Workflow > TDD Issue Test Tags.
Scenario: Config remains unchanged when second set_value fails during server_connect
Given a fresh config directory for atomic write test
And the config has pre-existing server values
When I invoke server_connect but set_value fails on the second call
Then an error should have been raised during server_connect
And the config should have rolled back server.url to its original value
And the config should still have the original server.namespace
And the config should still have the original server.tls-verify
Scenario: Config remains unchanged when third set_value fails during server_connect
Given a fresh config directory for atomic write test
And the config has pre-existing server values
When I invoke server_connect but set_value fails on the third call
Then an error should have been raised during server_connect
And the config should have rolled back server.url to its original value
And the config should have rolled back server.namespace to its original value
And the config should still have the original server.tls-verify
Scenario: No partial server.url persisted when namespace write raises
Given a fresh config directory for atomic write test
And the config has no server values
When I invoke server_connect but set_value fails on the second call
Then an error should have been raised during server_connect
And the config should not contain a server.url value