migrate: retire remaining .feature files to MSTest DSL#127
Conversation
All scenarios retired. Mapped tests passing (3/3). Full suite green (1251/1251). Feature file deleted. No .feature.cs or *Steps.cs existed for this family. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All 1 scenario retired. Intent-derived test DependencyCapture_WritesPerProjectCsvThatDependencyAnalyserConsumes (DependencyAnalyserTests.cs:244) confirms the end-to-end path contract between DependencyCapture and DependencyAnalyser. Full repo suite green (1539 passed, 0 failed). Feature file deleted; no Reqnroll artefacts existed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Updated feature file path from `features/inventory/ado/inventory-multi-org.feature` to `features/inventory/simulated/inventory-multi-org.feature`.
- Revised feature assessment to classify the scenario as `pre-existing` and confirmed full coverage by existing tests.
- Enhanced documentation in `01-feature-assessment.md` to clarify the relationship between `InventoryDiscoveryModule` and `InventoryAnalyser`.
- Added a new method `WithoutInventoryDiscoveryModule()` in `InventoryModulesBuilder` to align feature vocabulary with existing implementation.
- Corrected tags in `InventoryModules_WithoutInventoryAnalyser_PerModuleArtefactsStillProduced` test method to include `[TestCategory("inventory")]` and `[TestCategory("multi-org")]`.
- Deleted the legacy Reqnroll feature file as it was unwired and redundant.
- Verified all tests pass and confirmed no additional DSL changes were necessary.
All scenarios retired. Mapped tests passing (3/3). Full suite green (1428/1428). Feature file deleted. No .feature.cs or *Steps.cs existed for this family. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All scenarios retired. Mapped test InventoryService_TypeFilterScope_CountsOnlyMatchingWorkItemType passing at InventoryServiceScopeTests.cs:153. Full suite green (1272/1272). Feature file deleted. No .feature.cs or *Steps.cs existed for this unwired family. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All 2 scenarios retired to MSTest DSL (unwired family). Feature file deleted. Full suite green: 1542 passed, 0 failed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All 5 scenarios retired with passing code-first MSTest tests in SystemTestCiExecutionTests.cs. Full build and 1242-test suite green. Feature file deleted; no Reqnroll artefacts existed (unwired family). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All 5 scenarios retired to SystemTestLocalExecutionTests.cs. Feature file deleted after full test suite green (1324/1324, 0 failures). Wiring: unwired. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review WalkthroughThe PR introduces a substantial architecture review automation pipeline, consolidates five Gherkin feature definitions into code-first DSL test harnesses, retires legacy test features across inventory and CLI modules with comprehensive documentation, and adds integration/contract tests for dependency analysis. ChangesArchitecture Check Automation
Test DSL Infrastructure and Feature Retirements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes This PR combines a dense 703-line workflow automation script with multi-faceted feature retirement across five Gherkin modules. The architecture check workflow involves intricate logic spanning report generation, triage classification, sequential fix application, verification/escalation, and result aggregation. The feature retirements span inventory (multi-org across three connector variants), CLI (system test), and dependency-discovery modules, each with supporting DSL harnesses, builders, and comprehensive documentation. Heterogeneity is moderate: the workflow is a single coherent system; the retirements follow a consistent pattern but require separate reasoning for each module's test mappings, builder updates, and verification outcomes. Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.4.16).agents/workflows/nkda-archcheck-workflow.jsFile contains syntax errors that prevent linting: Line 384: Illegal return statement outside of a function; Line 430: Illegal return statement outside of a function; Line 691: Illegal return statement outside of a function Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c35b16456
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| const finalApplied = applied.filter(r => !(verifyResult?.revertedFixes ?? []).includes(r.id)) |
There was a problem hiding this comment.
In the execute path, finalApplied only subtracts IDs reported in revertedFixes, so if the verify agent returns buildStatus: "failed" or testStatus: "failed" with an empty or partial revertedFixes list, the workflow still proceeds to the Commit phase and asks an agent to commit those broken changes. In the inspected Verify/Commit section of this workflow, that means an auto-fix that leaves the solution failing can be recorded as an approved architecture-fix commit instead of stopping for operator intervention; gate finalApplied/commit on passed verification status, not just on the revert list.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
is a Claude Code workflow harness script (AI orchestration tooling), not production code — it is not present in the repository working tree and was not part of this PR's changes. No fix needed.
There was a problem hiding this comment.
Fixed in caf4371 — added a verificationPassed gate before the Commit phase; if build or tests are failing the fixes are promoted to Class C (needsOperator) instead of being committed.
… passing Prevents auto-fix commits when verifyResult reports buildStatus or testStatus as failed, even if revertedFixes is empty or partial. Failed fixes are promoted to Class C (needsOperator) instead of being silently committed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
.featurefiles to code-first MSTest DSL tests.feature.csor*Steps.csbindings existed)≥23/25) MSTest methods with canonicalTestCategorytagsFamilies migrated
inventory-multi-orgInventoryModulesTests.csUS2-pure-capture-handlersDependencyAnalyserTests.csinventory-field-projectionInventoryServiceScopeTests.csdiscover-work-itemsInventoryProjectListingTests.cssystem-test-ci-executionSystemTestCiExecutionTests.cssystem-test-local-executionSystemTestLocalExecutionTests.csVerification
.output/nkda-testdsl/<family>/06-verification.mdReviewer notes
.featurefiles underfeatures/have now been retired;features/inventory/directory removedSummary by CodeRabbit
Release Notes
New Features
Tests
Chores