From 45e72ca5638d12bd9852b569c02b47d99bca163a Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Tue, 2 Jun 2026 13:25:31 -0500 Subject: [PATCH] PDX-502: feat(mcp): add context-aware comparisonType enum validator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/mcp.md | 1 + scripts/mcp-smoke.cjs | 15 ++++ src/mcp/rules/comparisonTypeSets.ts | 49 +++++++++++++ src/mcp/tools/testCaseValidate.ts | 98 ++++++++++++++++++++++++++ test/unit/mcp/testCaseValidate.test.ts | 88 +++++++++++++++++++++++ 5 files changed, 251 insertions(+) create mode 100644 src/mcp/rules/comparisonTypeSets.ts diff --git a/docs/mcp.md b/docs/mcp.md index deef2fb..2a9c9d3 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1048,6 +1048,7 @@ Validates an XML test case for schema correctness (validity score) and best prac - **UI-TARGET-001** — A UiWithScreen or UiWithRow `target` argument uses the wrong XML class (e.g. `class="value"`). Must be `class="uiTarget"` or the screen binding is silently ignored at runtime. - **UI-LOCATOR-001** — A locator-bearing UI step (`UiDoAction`, `UiAssert`, `UiRead`, `UiFill`) has a `locator` argument that uses the wrong XML class. Must be `class="uiLocator"` or Provar cannot resolve the element. - **SETVALUES-STRUCTURE-001** (ERROR) — A `SetValues` step's `values` argument uses `class="value"` (plain string) instead of `class="valueList"` with `` children. This causes an immediate `ClassCastException` at runtime. +- **COMPARISON-TYPE-001** (ERROR) — A `comparisonType` value is used outside the subset its step type allows. `comparisonType` is a single Provar enum but each step type accepts only a subset: **AssertValues** accepts the 16-value set (`EqualTo, NotEqualTo, GreaterThan, GreaterThanOrEqualTo, LessThan, LessThanOrEqualTo, IsPresent, IsEmpty, Matches, NotMatches, Contains, NotContains, StartsWith, NotStartsWith, EndsWith, NotEndsWith`); a **UI Assert** (`uiAttributeAssertion`) accepts only the 6-value set (`EqualTo, Contains, StartsWith, EndsWith, Matches, None`). A value outside the step's subset (e.g. `NotEqualTo` on a UI Assert) is load-blocking — the whole test case fails to load at runtime with `IllegalArgumentException: No enum constant com.provar.core.model.base.java.ComparisonType.`. This local check runs even offline / in `local_fallback`, so the load-blocker is caught without the Quality Hub back-end. Only literal `comparisonType` values are checked; variable / compound references are skipped. See [`provar://docs/step-reference`](#resources) for the full step-scoped tables. - **UI-NEST-STRUCT-001** (severity `major`, weight 7, category `XMLSchema`) — A UI action step (`UiDoAction`, `UiAssert`, `UiRead`, `UiFill`, `UiNavigate`, `UiWithRow`, or `UiHandleAlert`) is emitted outside a screen ancestor. To pass, every UI action must descend from a `UiWithScreen` or `UiWithRow` `apiCall` through a `` path. Control-flow wrappers (`If`/`ForEach`/`DoWhile`/`WaitFor`/`Switch`) between the screen ancestor and the UI action are allowed; steps inside `` are exempt (disabled / settings blocks). One violation is emitted per offending step, so `(rule_id, test_item_id)` de-duplicates cleanly against the Quality Hub API. Provar IDE cannot bind flat-emitted UI actions to a screen context and they will not render in the editor. Wrap each offending step in the canonical chain: ```xml diff --git a/scripts/mcp-smoke.cjs b/scripts/mcp-smoke.cjs index ae54a7f..d1ccc83 100644 --- a/scripts/mcp-smoke.cjs +++ b/scripts/mcp-smoke.cjs @@ -194,6 +194,21 @@ async function runTests() { // ── 7. provar_testcase_validate ─────────────────────────────────────────── if (inGroup('validation')) await callTool('provar_testcase_validate', { content: '' }); + // ── 7b. provar_testcase_validate — COMPARISON-TYPE-001 (load-blocking enum) ─ + // Drives the context-aware comparisonType enum validator: NotEqualTo is valid + // for AssertValues but load-blocking on a UI Assert (uiAttributeAssertion). + // The smoke framework counts any JSON-RPC response as PASS; this just keeps the + // ERROR-tier code path exercised on every run. + if (inGroup('validation')) + await callTool('provar_testcase_validate', { + content: + '' + + '' + + '' + + '' + + '', + }); + // ── 8. provar_testsuite_validate ────────────────────────────────────────── if (inGroup('validation')) await callTool('provar_testsuite_validate', { suite_name: 'SmokeTestSuite' }); diff --git a/src/mcp/rules/comparisonTypeSets.ts b/src/mcp/rules/comparisonTypeSets.ts new file mode 100644 index 0000000..cb5a6d2 --- /dev/null +++ b/src/mcp/rules/comparisonTypeSets.ts @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +/** + * Canonical, step-scoped `comparisonType` subsets — the single source of truth + * shared by the docs (PROVAR_TEST_STEP_REFERENCE.md / mcp.md) and the local + * `comparisonType` enum validator (testCaseValidate.ts). + * + * `comparisonType` is a single Provar enum + * (`com.provar.core.model.base.java.ComparisonType`), but each step type accepts + * only a SUBSET. A value used outside the subset its step type allows is + * load-blocking: the whole test case fails to load at runtime with + * `IllegalArgumentException: No enum constant ...ComparisonType.`. + * + * Both subsets are confirmed from Provar-Automation-authored testcases (created + * directly in the product). Do NOT hand-duplicate these lists elsewhere — import + * from here. + */ + +/** AssertValues (`assertValuesComparison`) — the 16-value subset. */ +export const ASSERT_VALUES_COMPARISON_TYPES = [ + 'EqualTo', + 'NotEqualTo', + 'GreaterThan', + 'GreaterThanOrEqualTo', + 'LessThan', + 'LessThanOrEqualTo', + 'IsPresent', + 'IsEmpty', + 'Matches', + 'NotMatches', + 'Contains', + 'NotContains', + 'StartsWith', + 'NotStartsWith', + 'EndsWith', + 'NotEndsWith', +] as const; + +/** UI Assert (`uiAttributeAssertion`) — the narrower 6-value subset. */ +export const UI_ASSERT_COMPARISON_TYPES = ['EqualTo', 'Contains', 'StartsWith', 'EndsWith', 'Matches', 'None'] as const; + +export const ASSERT_VALUES_COMPARISON_TYPE_SET: ReadonlySet = new Set(ASSERT_VALUES_COMPARISON_TYPES); + +export const UI_ASSERT_COMPARISON_TYPE_SET: ReadonlySet = new Set(UI_ASSERT_COMPARISON_TYPES); diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index 52366d0..1732503 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/src/mcp/tools/testCaseValidate.ts @@ -38,6 +38,12 @@ import { } from '../utils/validationDiff.js'; import { resolveTestCasePlanMode, type TestCasePlanMode } from '../utils/testCasePlanMode.js'; import { WARNING_CODES, formatWarning } from '../utils/warningCodes.js'; +import { + ASSERT_VALUES_COMPARISON_TYPES, + UI_ASSERT_COMPARISON_TYPES, + ASSERT_VALUES_COMPARISON_TYPE_SET, + UI_ASSERT_COMPARISON_TYPE_SET, +} from '../rules/comparisonTypeSets.js'; import { runBestPractices } from './bestPracticesEngine.js'; import { desc } from './descHelper.js'; import { UI_SCREEN_CONTAINER_API_IDS, UI_LOCATOR_BEARING_API_IDS } from './uiActionApiIds.js'; @@ -518,6 +524,12 @@ export function validateTestCase( validateApiCall(call, issues); } + // COMPARISON-TYPE-001: context-aware comparisonType enum check (load-blocking). + // Walks the whole test case so nested steps (e.g. a UiAssert inside a + // UiWithScreen substeps clause, or an AssertValues inside an If/ForEach) are + // covered too, not just top-level apiCalls. + validateComparisonTypes(tc, issues); + // VAR-REF-001 / VAR-REF-002: detect {VarName} tokens inside valueClass="string" elements. // Provar does not interpolate {…} tokens in plain string values at runtime — they must use // class="variable" (pure reference) or class="compound" (embedded in surrounding text). @@ -747,6 +759,92 @@ function validateApiCallArgs( } } +/** Recursively collect every node stored under `key` anywhere in the tree. */ +function collectNodesByKey(node: unknown, key: string, out: Array>): void { + if (Array.isArray(node)) { + for (const item of node) collectNodesByKey(item, key, out); + return; + } + if (node === null || typeof node !== 'object') return; + const obj = node as Record; + for (const [k, v] of Object.entries(obj)) { + if (k === key) { + if (Array.isArray(v)) { + for (const item of v) if (item !== null && typeof item === 'object') out.push(item as Record); + } else if (v !== null && typeof v === 'object') { + out.push(v as Record); + } + } + collectNodesByKey(v, key, out); + } +} + +/** Extract the literal text of a parsed `` node, or undefined if not a literal. */ +function literalValueText(valueNode: Record | undefined): string | undefined { + if (valueNode == null) return undefined; + // Only validate literal string values — skip variable / compound references, + // whose runtime value cannot be checked statically. + const valClass = valueNode['@_class'] as string | undefined; + if (valClass !== undefined && valClass !== 'value') return undefined; + const text = valueNode['#text']; + if (typeof text === 'string') return text; + if (typeof text === 'number') return String(text); + return undefined; +} + +/** + * COMPARISON-TYPE-001 — context-aware `comparisonType` enum validation. + * + * `comparisonType` is a single Provar enum, but each step type accepts only a + * SUBSET. A value outside its step's subset is load-blocking (the test case + * fails to load with `IllegalArgumentException: No enum constant + * ...ComparisonType.`), so this is emitted at ERROR tier — not as a + * best-practices quality warning — to mirror the runtime failure offline. + */ +function validateComparisonTypes(tc: Record, issues: ValidationIssue[]): void { + // AssertValues steps → the 16-value AssertValues subset. + const apiCalls: Array> = []; + collectNodesByKey(tc, 'apiCall', apiCalls); + for (const call of apiCalls) { + const apiId = call['@_apiId'] as string | undefined; + if (!apiId || !apiId.includes('AssertValues')) continue; + const cmpArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'comparisonType'); + if (!cmpArg) continue; + const cmp = literalValueText(cmpArg['value'] as Record | undefined); + if (cmp === undefined || cmp === '') continue; + if (!ASSERT_VALUES_COMPARISON_TYPE_SET.has(cmp)) { + const stepName = (call['@_name'] as string | undefined) ?? '(unnamed)'; + issues.push({ + rule_id: 'COMPARISON-TYPE-001', + severity: 'ERROR', + message: `AssertValues step "${stepName}" uses comparisonType="${cmp}", which is not a valid AssertValues comparison — this is load-blocking (IllegalArgumentException: No enum constant com.provar.core.model.base.java.ComparisonType.${cmp}).`, + applies_to: 'apiCall', + suggestion: `Use one of the AssertValues comparison types: ${ASSERT_VALUES_COMPARISON_TYPES.join(', ')}.`, + }); + } + } + + // UI Assert attribute assertions → the narrower 6-value UI subset. + const uiAsserts: Array> = []; + collectNodesByKey(tc, 'uiAttributeAssertion', uiAsserts); + for (const node of uiAsserts) { + const cmp = node['@_comparisonType'] as string | undefined; + if (cmp === undefined || cmp === '') continue; + if (!UI_ASSERT_COMPARISON_TYPE_SET.has(cmp)) { + const attr = (node['@_attributeName'] as string | undefined) ?? '(unknown attribute)'; + issues.push({ + rule_id: 'COMPARISON-TYPE-001', + severity: 'ERROR', + message: `UI Assert attribute assertion (attributeName="${attr}") uses comparisonType="${cmp}", which is not valid for a UI Assert (uiAttributeAssertion) — this is load-blocking (IllegalArgumentException: No enum constant com.provar.core.model.base.java.ComparisonType.${cmp}).`, + applies_to: 'apiCall', + suggestion: `Use one of the UI Assert comparison types: ${UI_ASSERT_COMPARISON_TYPES.join( + ', ' + )}. The negation/relational operators (e.g. NotEqualTo) are valid only in AssertValues steps; to negate a UI comparison, invert the assertion logic.`, + }); + } + } +} + function finalize( issues: ValidationIssue[], testCaseId: string | null, diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index de374bf..9c19916 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -12,6 +12,10 @@ import { validateTestCaseXml, } from '../../../src/mcp/tools/testCaseValidate.js'; import type { ServerConfig } from '../../../src/mcp/server.js'; +import { + ASSERT_VALUES_COMPARISON_TYPES, + UI_ASSERT_COMPARISON_TYPES, +} from '../../../src/mcp/rules/comparisonTypeSets.js'; import { qualityHubClient, QualityHubAuthError, @@ -1459,3 +1463,87 @@ describe('provar_testcase_validate description', () => { ); }); }); + +describe('COMPARISON-TYPE-001: context-aware comparisonType enum validator', () => { + const G1 = '6ba7b810-9dad-4000-8000-00c04fd430c8'; + + const assertValuesTC = (cmp: string): string => + ` + + + + + + ${cmp} + + + + +`; + + const uiAssertTC = (cmp: string): string => + ` + + + + + + + + + + + + + + + + +`; + + const hasComparisonError = (r: ReturnType): boolean => + r.issues.some((i) => i.rule_id === 'COMPARISON-TYPE-001' && i.severity === 'ERROR'); + + it('passes every valid AssertValues constant (incl. NotEqualTo) on an AssertValues step', () => { + for (const cmp of ASSERT_VALUES_COMPARISON_TYPES) { + const r = validateTestCase(assertValuesTC(cmp)); + assert.ok(!hasComparisonError(r), `AssertValues comparisonType="${cmp}" must not fire COMPARISON-TYPE-001`); + } + }); + + it('passes every valid UI Assert constant on a UI Assert step', () => { + for (const cmp of UI_ASSERT_COMPARISON_TYPES) { + const r = validateTestCase(uiAssertTC(cmp)); + assert.ok(!hasComparisonError(r), `UI Assert comparisonType="${cmp}" must not fire COMPARISON-TYPE-001`); + } + }); + + it('fires ERROR for NotEqualTo on a UI Assert step (valid for AssertValues, not UI)', () => { + // Context-sensitive: the same value passes on AssertValues but is load-blocking on a UI Assert. + assert.ok(!hasComparisonError(validateTestCase(assertValuesTC('NotEqualTo')))); + const r = validateTestCase(uiAssertTC('NotEqualTo')); + assert.ok(hasComparisonError(r), 'NotEqualTo on a UI Assert must fire COMPARISON-TYPE-001'); + assert.equal(r.is_valid, false, 'a load-blocking comparisonType must make the test case invalid'); + }); + + it('fires ERROR for a constant in neither set (NotEqualToo) on both step types', () => { + const av = validateTestCase(assertValuesTC('NotEqualToo')); + assert.ok(hasComparisonError(av), 'NotEqualToo on AssertValues must fire COMPARISON-TYPE-001'); + const ui = validateTestCase(uiAssertTC('NotEqualToo')); + assert.ok(hasComparisonError(ui), 'NotEqualToo on a UI Assert must fire COMPARISON-TYPE-001'); + }); + + it('does not fire for a variable-reference comparisonType (cannot be checked statically)', () => { + const xml = ` + + + + + + + + +`; + assert.ok(!hasComparisonError(validateTestCase(xml))); + }); +});