PDX-502: add context-aware comparisonType enum validator#199
Merged
Conversation
RCA: A comparisonType used outside the subset its step type allows (e.g. NotEqualTo on a UI Assert) is load-blocking, but nothing caught it locally — offline / local_fallback sessions had no comparisonType coverage at all, so the load-blocker slipped through. Fix: Add a context-aware COMPARISON-TYPE-001 ERROR-tier check in testCaseValidate that validates AssertValues against the 16-value subset and uiAttributeAssertion against the 6-value subset, sourced from a single shared comparisonTypeSets module; deep-walks nested steps. Adds unit tests, a smoke entry, and the docs/mcp.md error-code entry.
Quality Orchestrator🟢 LOW · 🧪 Tests to Run · Running 1 of 54 tests
▶ Run commandnpx vitest run \
unit/mcp/testCaseValidate.test.ts
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a load-blocking, context-aware validator for comparisonType values in test case XML so invalid enum constants are caught locally (including local_fallback), matching Provar’s runtime load failure behavior.
Changes:
- Introduces
COMPARISON-TYPE-001(ERROR) and validatescomparisonTypeagainst step-specific allowed subsets (AssertValues vs UI Assert). - Centralizes the allowed subsets in a single shared module (
comparisonTypeSets.ts) and reuses them across validator + tests. - Updates docs and smoke coverage to keep the new load-blocking path exercised.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/mcp/rules/comparisonTypeSets.ts |
Adds canonical, shared step-scoped comparisonType subsets and set helpers. |
src/mcp/tools/testCaseValidate.ts |
Implements deep-walk validation emitting COMPARISON-TYPE-001 at ERROR tier. |
test/unit/mcp/testCaseValidate.test.ts |
Adds unit tests covering both allowed sets, cross-context invalid values, and variable-reference skip behavior. |
scripts/mcp-smoke.cjs |
Adds a smoke invocation that exercises the new ERROR-tier validator path. |
docs/mcp.md |
Documents COMPARISON-TYPE-001 behavior, scope, and allowed value sets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
comparisonTypeenum validator that fires at ERROR / load-blocking tier (not a best-practices quality warning), so the load-blocker is caught locally even offline / inlocal_fallback— previously only the Quality Hub back-end covered it.COMPARISON-TYPE-001(ERROR), context-aware:assertValuesComparison) validated against the 16-value subset.uiAttributeAssertion) validated against the 6-value subset.NotEqualToon a UI Assert) now makes the test caseis_valid: false, mirroring the runtimeIllegalArgumentException: No enum constant …ComparisonType.<value>.src/mcp/rules/comparisonTypeSets.tsholds both subsets (same canonical sets confirmed in PDX-501); the validator and tests import from it — no hand-duplicated lists.UiWithScreensubsteps clause, an AssertValues insideIf/ForEach) are covered too. Variable / compoundcomparisonTypereferences are skipped (cannot be checked statically).Implementation note
The ticket suggested registering the check in the best-practices
VALIDATOR_REGISTRY. BP-engine violations are quality-tier (critical/major/minor/info) and never flipis_valid, so they cannot express the load-blocking ERROR tier the ticket requires ("not a quality warning"). The check is therefore implemented on the schema-validator ERROR path intestCaseValidate.ts— exactly how the other load-blocking rules (SETVALUES-STRUCTURE-001,UI-TARGET-001) are handled. Behaviour matches the acceptance criteria.Jira
https://provartesting.atlassian.net/browse/PDX-502
Test plan
Tests (per ticket acceptance criteria)
NotEqualTo) PASSES on an AssertValues stepNotEqualToon a UI Assert step fires ERROR (and flipsis_valid)NotEqualToo) fires ERROR on both step typescomparisonTypedoes not fire (negative case)Changes
src/mcp/rules/comparisonTypeSets.ts(new): canonical step-scoped subsets.src/mcp/tools/testCaseValidate.ts:validateComparisonTypesdeep-walk +COMPARISON-TYPE-001ERROR issues.docs/mcp.md:COMPARISON-TYPE-001error-code entry.scripts/mcp-smoke.cjs: smoke entry exercising the UI-Assert load-blocker path (expected count auto-increments).test/unit/mcp/testCaseValidate.test.ts: 5 new tests.