diff --git a/docs/mcp.md b/docs/mcp.md index fa3dce8..3366c61 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -637,6 +637,18 @@ Generates an XML test case skeleton with UUID v4 guids and sequential `testItemI - Omit or use a `sf:` URI → flat Salesforce step structure (existing behaviour) - `ui:pageobject:target?pageId=pageobjects.Page` → wraps all steps in a `UiWithScreen` element (testItemId=1); substeps clause at testItemId=2; inner steps start at testItemId=3 +**Argument XML conventions** (automatically applied by the generator): + +| Argument key / value pattern | Emitted XML class | API context | +| ------------------------------------ | ----------------------------- | ----------------------------------- | +| `target` key | `class="uiTarget"` | UiWithScreen, UiWithRow | +| `locator` key | `class="uiLocator"` | UiDoAction, UiAssert | +| Value matches `{VarName}` or `{A.B}` | `class="variable"` + `` | Any step | +| SetValues attributes | `class="valueList"/` | SetValues only | +| All other values | `class="value" valueClass="string"` | Any step | + +AssertValues uses **flat** argument structure (`expectedValue`, `actualValue`, `comparisonType`) — not the `valueList`/namedValues format. + **Input** | Parameter | Type | Required | Description | @@ -700,6 +712,10 @@ Validates an XML test case for schema correctness (validity score) and best prac - **DATA-001** — `testCase` declares a `` element. CLI standalone execution does not bind CSV column variables; steps using variable references will resolve to null. Use `SetValues` (Test scope) steps instead, or add the test to a test plan. - **ASSERT-001** — An `AssertValues` step uses the `argument id="values"` (namedValues) format, which is designed for UI element attribute assertions. For Apex/SOQL result or variable comparisons this silently passes as `null=null`. Use separate `expectedValue`, `actualValue`, and `comparisonType` arguments instead. +- **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 UiDoAction or UiAssert `locator` argument 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. +- **VAR-REF-001** — An argument value looks like a variable reference (`{VarName}` or `{Obj.Field}`) but is stored as `class="value" valueClass="string"`. Provar will treat it as a literal string, not resolve the variable. Replace with `class="variable"` and `` elements. --- diff --git a/package.json b/package.json index 8c47f57..8f45989 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@provartesting/provardx-cli", "description": "A plugin for the Salesforce CLI to orchestrate testing activities and report quality metrics to Provar Quality Hub", - "version": "1.5.0-beta.11", + "version": "1.5.0-beta.12", "mcpName": "io.github.ProvarTesting/provar", "license": "BSD-3-Clause", "plugins": [ diff --git a/scripts/mcp-smoke.cjs b/scripts/mcp-smoke.cjs index 06b709f..39bdf46 100644 --- a/scripts/mcp-smoke.cjs +++ b/scripts/mcp-smoke.cjs @@ -248,7 +248,14 @@ async function runTests() { // ── 30. provar.testrun.rca ─────────────────────────────────────────────── await callTool('provar.testrun.rca', { project_path: TMP }); - // ── 31. provar.testplan.add-instance ───────────────────────────────────── + // ── 31. provar.testplan.create ──────────────────────────────────────────── + // TMP is not a Provar project → NOT_A_PROJECT result + await callTool('provar.testplan.create', { + project_path: TMP, + plan_name: 'SmokePlan', + }); + + // ── 32. provar.testplan.add-instance ───────────────────────────────────── // TMP is not a Provar project → NOT_A_PROJECT result await callTool('provar.testplan.add-instance', { project_path: TMP, @@ -256,24 +263,24 @@ async function runTests() { plan_name: 'SmokePlan', }); - // ── 32. provar.testplan.create-suite ───────────────────────────────────── + // ── 33. provar.testplan.create-suite ───────────────────────────────────── await callTool('provar.testplan.create-suite', { project_path: TMP, plan_name: 'SmokePlan', suite_name: 'SmokeSuite', }); - // ── 33. provar.testplan.remove-instance ────────────────────────────────── + // ── 34. provar.testplan.remove-instance ────────────────────────────────── await callTool('provar.testplan.remove-instance', { project_path: TMP, instance_path: 'plans/SmokePlan/SmokeSuite/smoke.testinstance', }); - // ── 34. provar.nitrox.discover ──────────────────────────────────────────── + // ── 35. provar.nitrox.discover ──────────────────────────────────────────── // TMP has no .testproject → empty projects list, no crash await callTool('provar.nitrox.discover', { search_roots: [TMP] }); - // ── 35. provar.nitrox.validate ──────────────────────────────────────────── + // ── 36. provar.nitrox.validate ──────────────────────────────────────────── // Minimal valid root component → score 100 await callTool('provar.nitrox.validate', { content: JSON.stringify({ @@ -376,8 +383,8 @@ async function runTests() { // ---------------------------------------------------------------------------- server.on('close', () => { clearTimeout(overallTimer); - // initialize + tools/list + 39 tools + prompts/list + 8 prompts/get (setup excluded from default count) - const TOTAL_EXPECTED = 50 + (INCLUDE_SETUP ? 1 : 0); + // initialize + tools/list + 40 tools + prompts/list + 8 prompts/get (setup excluded from default count) + const TOTAL_EXPECTED = 51 + (INCLUDE_SETUP ? 1 : 0); let passed = 0; let failed = 0; diff --git a/server.json b/server.json index 781cc2a..c23758f 100644 --- a/server.json +++ b/server.json @@ -14,12 +14,12 @@ "url": "https://github.com/ProvarTesting/provardx-cli", "source": "github" }, - "version": "1.5.0-beta.11", + "version": "1.5.0-beta.12", "packages": [ { "registryType": "npm", "identifier": "@provartesting/provardx-cli", - "version": "1.5.0-beta.11", + "version": "1.5.0-beta.12", "transport": { "type": "stdio" }, diff --git a/src/mcp/tools/testCaseGenerate.ts b/src/mcp/tools/testCaseGenerate.ts index e053cf9..d7dff52 100644 --- a/src/mcp/tools/testCaseGenerate.ts +++ b/src/mcp/tools/testCaseGenerate.ts @@ -69,6 +69,17 @@ function buildStepWarnings(steps: Array<{ api_id: string }>): string[] { ); } + // D7: Cleanup steps placed after a potential failure point are skipped when stopOnError=false. + if (resolvedIds.includes(SHORTHAND_TO_FQID['ApexDeleteObject'] ?? '')) { + warnings.push( + 'ApexDeleteObject detected (likely cleanup): with stopOnError=false Provar skips all steps after ' + + 'the first failure, so cleanup steps placed at the end of the test will NOT run when an earlier ' + + 'step fails — leaving orphaned records in the org. ' + + 'Wrap cleanup in a Provar TearDown callable, or place create/delete inside the same UiWithScreen ' + + 'clause so both run as a unit regardless of failure.' + ); + } + return warnings; } @@ -89,10 +100,19 @@ const StepSchema = z.object({ .record(z.string()) .default({}) .describe( - 'Step argument values as key/value pairs. Written as ' + - 'inside the element — the format Provar runtime requires. ' + + 'Step argument values as key/value pairs. Written as . ' + 'Do NOT rely on XML attributes on ; the runtime silently ignores them. ' + - 'Example: { "connectionName": "MyOrg", "objectApiName": "Opportunity" }' + 'Special value conventions (applied automatically by the generator): ' + + '(1) Variable references: wrap the name in braces, e.g. "{MyVar}" → emitted as class="variable" . ' + + ' Dotted paths are also supported: "{Obj.Field}" → two elements. ' + + '(2) SetValues: pass each variable name and its value as a flat key/value pair; ' + + ' the generator wraps them in ... automatically. ' + + ' Example: { "testCaseName": "TC_New", "testType": "Acceptance testing" } ' + + '(3) AssertValues: pass assertion arguments as flat key/value pairs; emitted as flat elements, NOT wrapped in valueList/namedValues. ' + + '(4) target argument (UiWithScreen / UiWithRow): pass the sf:ui:target or ui:pageobject:target URI; ' + + ' emitted as class="uiTarget" uri="...". ' + + '(5) locator argument (UiDoAction / UiAssert): pass the locator URI; emitted as class="uiLocator" uri="...". ' + + 'All other string values use class="value" valueClass="string".' ), }); @@ -111,6 +131,14 @@ const TOOL_DESCRIPTION = [ 'ApexReadObject requires field names in attributes; omitting them produces MALFORMED_QUERY. Prefer ApexSoqlQuery.', 'AssertValues on SOQL results: index paths like "ResultList[0].Field" are not supported.', 'Use ForEach to iterate the result list, or SetValues to extract a field into a variable first.', + 'SetValues: pass named variable values as flat key/value pairs in attributes; ' + + 'the generator wraps them in ... automatically.', + 'AssertValues: pass assertion values as flat key/value argument pairs; emitted as flat arguments, NOT wrapped in namedValues. ' + + 'If AssertValues uses namedValues-shaped content, validation reports warning ASSERT-001.', + 'Variable references: pass values as "{VarName}" (braces); emitted as class="variable" .', + 'target argument (UiWithScreen/UiWithRow): pass the URI value; emitted as class="uiTarget" uri="...".', + 'locator argument (UiDoAction/UiAssert): pass the URI value; emitted as class="uiLocator" uri="...".', + 'Cleanup warning: ApexDeleteObject steps near end of test will be skipped if an earlier step fails (stopOnError=false). Use a TearDown callable.', 'Validation: when validate_after_edit=true (default) the response includes a validation field and returns TESTCASE_INVALID if the generated XML fails structural checks.', 'Grounding: call provar.qualityhub.examples.retrieve before generating to get corpus examples for the scenario — correct XML structure for the step types you need.', ].join(' '); @@ -239,20 +267,73 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig // ── XML builder ─────────────────────────────────────────────────────────────── -function buildArgumentsXml(attributes: Record, baseIndent = ' '): string { +// Build the element for a single argument (D2/D4 aware). +// inNamedValues: when true (inside SetValues namedValues), skip uiTarget/uiLocator dispatch. +// apiId: resolved API ID used to restrict key-name dispatch to the correct UI APIs. +function buildArgumentValue(key: string, val: string, indent: string, inNamedValues = false, apiId = ''): string { + // D4: {VarName} or {Obj.Field} → class="variable" with elements. + const varMatch = /^\{([\w.]+)\}$/.exec(val); + if (varMatch) { + const pathElements = varMatch[1] + .split('.') + .map((p) => `${indent} `) + .join('\n'); + return `${indent}\n${pathElements}\n${indent}`; + } + if (!inNamedValues) { + // D2: 'target' argument → class="uiTarget" (only for UiWithScreen / UiWithRow). + if (key === 'target' && (apiId.includes('UiWithScreen') || apiId.includes('UiWithRow'))) { + return `${indent}`; + } + // D2: 'locator' argument → class="uiLocator" (only for UiDoAction / UiAssert). + if (key === 'locator' && (apiId.includes('UiDoAction') || apiId.includes('UiAssert'))) { + return `${indent}`; + } + } + return `${indent}${escapeXmlContent(val)}`; +} + +function buildArgumentsXml(attributes: Record, baseIndent = ' ', apiId = ''): string { const entries = Object.entries(attributes); if (entries.length === 0) return ''; const argLines = entries - .map( - ([k, v]) => + .map(([k, v]) => { + const valueXml = buildArgumentValue(k, v, `${baseIndent} `, false, apiId); + return ( `${baseIndent}\n` + - `${baseIndent} ${escapeXmlContent(v)}\n` + + valueXml + '\n' + `${baseIndent}` - ) + ); + }) .join('\n'); return `\n${baseIndent}\n${argLines}\n${baseIndent}\n${baseIndent.slice(0, -2)}`; } +// D3: SetValues — all attributes become under a single 'values' argument. +function buildSetValuesXml(attributes: Record, baseIndent: string): string { + const entries = Object.entries(attributes); + if (entries.length === 0) return ''; + const i = (n: number): string => baseIndent + ' '.repeat(n); + const namedValueLines = entries + .map(([name, val]) => { + const valueXml = buildArgumentValue(name, val, `${i(3)} `, true); + return `${i(3)}\n${valueXml}\n${i(3)}`; + }) + .join('\n'); + return ( + `\n${i(0)}\n` + + `${i(0)}\n` + + `${i(1)}\n` + + `${i(2)}\n` + + namedValueLines + '\n' + + `${i(2)}\n` + + `${i(1)}\n` + + `${i(0)}\n` + + `${i(0)}\n` + + `${baseIndent.slice(0, -2)}` + ); +} + function buildFlatStepXml( step: { api_id: string; name: string; attributes: Record }, testItemId: number, @@ -260,7 +341,11 @@ function buildFlatStepXml( ): string { const guid = randomUUID(); const resolvedApiId = resolveApiId(step.api_id); - const argumentsXml = buildArgumentsXml(step.attributes, indent + ' '); + const baseIndent = indent + ' '; + // Use SetValues structure for any SetValues API (string-match mirrors the validator). + const argumentsXml = resolvedApiId.includes('SetValues') + ? buildSetValuesXml(step.attributes, baseIndent) + : buildArgumentsXml(step.attributes, baseIndent, resolvedApiId); if (argumentsXml) { return ( `${indent}\n '; return ( ` ${buildArgumentsXml({ target: targetUri }).trimEnd()}${clausesXml}` + ` name="With page" testItemId="1">${buildArgumentsXml({ target: targetUri }, ' ', wrapperApiId).trimEnd()}${clausesXml}` ); } diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index 607c9d3..8c36431 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/src/mcp/tools/testCaseValidate.ts @@ -312,9 +312,33 @@ export function validateTestCase(xmlContent: string, testName?: string): TestCas validateApiCall(call, issues); } + // VAR-REF-001 (gap in both local and quality-hub-agents backend): + // Detect {VarName} or {Obj.Field} literals stored as plain string values. + // Provar will pass these as-is to the API rather than resolving them as variable references. + const varLiteralRe = /]+valueClass="string"[^>]*>\{([\w.]+)\}<\/value>/g; + let varMatch: RegExpExecArray | null; + while ((varMatch = varLiteralRe.exec(xmlContent)) !== null) { + issues.push({ + rule_id: 'VAR-REF-001', + severity: 'WARNING', + message: `Argument value "{${varMatch[1]}}" looks like a variable reference but is stored as a plain string — Provar will not resolve it at runtime.`, + applies_to: 'argument', + suggestion: `Replace with . In provar.testcase.generate, use the {VarName} syntax in the attributes object — the generator converts it automatically.`, + }); + } + return finalize(issues, tcId, tcName, apiCalls.length, xmlContent, testName); } +/** Normalise fast-xml-parser's single-or-array representation of children. */ +function getArgList(call: Record): Array> { + const rawArgs = call['arguments'] as Record | undefined; + if (!rawArgs) return []; + const argRaw = rawArgs['argument']; + if (!argRaw) return []; + return (Array.isArray(argRaw) ? argRaw : [argRaw]) as Array>; +} + function validateApiCall(call: Record, issues: ValidationIssue[]): void { const callGuid = call['@_guid'] as string | undefined; const apiId = call['@_apiId'] as string | undefined; @@ -376,31 +400,99 @@ function validateApiCall(call: Record, issues: ValidationIssue[ }); } - // ASSERT-001: AssertValues using UI namedValues format instead of variable format - if (apiId?.includes('AssertValues')) { - const rawArgs = call['arguments'] as Record | undefined; - if (rawArgs) { - const argRaw = rawArgs['argument']; - const argList: Array> = !argRaw - ? [] - : Array.isArray(argRaw) - ? (argRaw as Array>) - : [argRaw as Record]; - const hasValuesArg = argList.some((a) => (a['@_id'] as string | undefined) === 'values'); - if (hasValuesArg) { + if (apiId) validateApiCallArgs(call, apiId, name, issues); +} + +function checkUiTarget(call: Record, apiId: string, stepName: string, issues: ValidationIssue[]): void { + const targetArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'target'); + if (!targetArg) return; + const valClass = (targetArg['value'] as Record | undefined)?.['@_class'] as string | undefined; + if (valClass && valClass !== 'uiTarget') { + const apiLabel = apiId.includes('UiWithRow') ? 'UiWithRow' : 'UiWithScreen'; + issues.push({ + rule_id: 'UI-TARGET-001', + severity: 'ERROR', + message: `${apiLabel} step "${stepName}" target argument uses class="${valClass}" — must be class="uiTarget".`, + applies_to: 'apiCall', + suggestion: + 'Emit the target as: or uri="ui:pageobject:target?pageId=...". ' + + 'In provar.testcase.generate the "target" attribute is converted automatically.', + }); + } +} + +function validateApiCallArgs( + call: Record, + apiId: string, + name: string | undefined, + issues: ValidationIssue[] +): void { + const stepName = name ?? '(unnamed)'; + + // UI-TARGET-001 (mirrors quality-hub-agents UI-SCREEN-TARGET-001): + // UiWithScreen / UiWithRow target argument must use class="uiTarget", not a plain string. + // A plain string causes: "Can not set IUiTargetValue field ... to java.lang.String" + if (apiId.includes('UiWithScreen') || apiId.includes('UiWithRow')) { + checkUiTarget(call, apiId, stepName, issues); + } + + // UI-LOCATOR-001 (local rule, no direct backend equivalent): + // UiDoAction / UiAssert locator argument must use class="uiLocator". + if (apiId.includes('UiDoAction') || apiId.includes('UiAssert')) { + const locatorArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'locator'); + if (locatorArg) { + const valClass = (locatorArg['value'] as Record | undefined)?.['@_class'] as string | undefined; + if (valClass && valClass !== 'uiLocator') { + issues.push({ + rule_id: 'UI-LOCATOR-001', + severity: 'ERROR', + message: `"${stepName}" locator argument uses class="${valClass}" — must be class="uiLocator".`, + applies_to: 'apiCall', + suggestion: + 'Emit the locator as: . ' + + 'In provar.testcase.generate the "locator" attribute is converted automatically.', + }); + } + } + } + + // SETVALUES-STRUCTURE-001 (mirrors quality-hub-agents SETVALUES-STRUCTURE-001): + // SetValues values argument must use class="valueList" with children. + // A plain string value causes an immediate ClassCastException at runtime. + if (apiId.includes('SetValues') && !apiId.includes('AssertValues')) { + const valuesArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'values'); + if (valuesArg) { + const valClass = (valuesArg['value'] as Record | undefined)?.['@_class'] as string | undefined; + if (valClass && valClass !== 'valueList') { issues.push({ - rule_id: 'ASSERT-001', - severity: 'WARNING', - message: `AssertValues step "${ - name ?? '(unnamed)' - }" uses namedValues format (argument id="values") — designed for UI element attribute assertions. For Apex/SOQL result or variable comparisons this silently passes as null=null.`, + rule_id: 'SETVALUES-STRUCTURE-001', + severity: 'ERROR', + message: `SetValues step "${stepName}" values argument uses class="${valClass}" — must use class="valueList" with children.`, applies_to: 'apiCall', suggestion: - 'Use separate expectedValue, actualValue, and comparisonType arguments for variable or Apex result comparisons.', + 'Wrap variable assignments in: ' + + 'value' + + '. In provar.testcase.generate pass each variable as a flat key/value pair ' + + 'in attributes — the generator builds the valueList structure automatically.', }); } } } + + // ASSERT-001: AssertValues using UI namedValues format instead of variable format + if (apiId.includes('AssertValues')) { + const hasValuesArg = getArgList(call).some((a) => (a['@_id'] as string | undefined) === 'values'); + if (hasValuesArg) { + issues.push({ + rule_id: 'ASSERT-001', + severity: 'WARNING', + message: `AssertValues step "${stepName}" uses namedValues format (argument id="values") — designed for UI element attribute assertions. For Apex/SOQL result or variable comparisons this silently passes as null=null.`, + applies_to: 'apiCall', + suggestion: + 'Use separate expectedValue, actualValue, and comparisonType arguments for variable or Apex result comparisons.', + }); + } + } } function finalize( diff --git a/src/mcp/tools/testPlanTools.ts b/src/mcp/tools/testPlanTools.ts index d2c37f7..716c402 100644 --- a/src/mcp/tools/testPlanTools.ts +++ b/src/mcp/tools/testPlanTools.ts @@ -57,6 +57,132 @@ function buildPlanItemXml(guid: string): string { ].join('\n'); } +// ── provar.testplan.create ──────────────────────────────────────────────────── + +export function registerTestPlanCreate(server: McpServer, config: ServerConfig): void { + server.tool( + 'provar.testplan.create', + [ + 'Create a new Provar test plan: makes the plans/{plan_name}/ directory and writes the root .planitem file.', + 'Use this before provar.testplan.create-suite or provar.testplan.add-instance, which both require the plan to already exist.', + 'Returns the guid assigned to the new plan, the plan directory path, and the .planitem path written.', + ].join(' '), + { + project_path: z.string().describe('Absolute path to the Provar project root (must contain a .testproject file)'), + plan_name: z.string().describe('Name of the new test plan (becomes the directory name under plans/)'), + overwrite: z + .boolean() + .optional() + .default(false) + .describe('Overwrite the .planitem file if the plan directory already exists (default: false)'), + dry_run: z + .boolean() + .optional() + .default(false) + .describe('Return what would be created without writing to disk (default: false)'), + }, + ({ project_path, plan_name, overwrite, dry_run }) => { + const requestId = makeRequestId(); + log('info', 'provar.testplan.create', { requestId, project_path, plan_name }); + + try { + assertPathAllowed(project_path, config.allowedPaths); + + const projectRoot = path.resolve(project_path); + + const testProjectFiles = fs.existsSync(projectRoot) + ? fs.readdirSync(projectRoot).filter((f) => f.endsWith('.testproject')) + : []; + if (testProjectFiles.length === 0) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify(makeError('NOT_A_PROJECT', `No .testproject file found in ${projectRoot}`, requestId)), + }, + ], + }; + } + + if (plan_name.includes('/') || plan_name.includes('\\') || path.isAbsolute(plan_name)) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify(makeError('INVALID_PLAN_NAME', `plan_name must be a simple directory name without path separators: "${plan_name}"`, requestId)), + }, + ], + }; + } + const planDir = path.join(projectRoot, 'plans', plan_name); + assertPathAllowed(planDir, config.allowedPaths); + const planItemPath = path.join(planDir, '.planitem'); + + if (!overwrite && fs.existsSync(planItemPath)) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify( + makeError( + 'PLAN_EXISTS', + `Plan already exists: ${planItemPath}. Set overwrite: true to replace the .planitem file.`, + requestId + ) + ), + }, + ], + }; + } + + const guid = randomUUID(); + const xmlContent = buildPlanItemXml(guid); + + if (!dry_run) { + fs.mkdirSync(planDir, { recursive: true }); + fs.writeFileSync(planItemPath, xmlContent, 'utf-8'); + } + + const response = { + requestId, + plan_dir: planDir, + planitem_path: planItemPath, + guid, + dry_run: dry_run ?? false, + created: !dry_run, + next_steps: dry_run + ? 'Review the plan structure, then call provar.testplan.create with dry_run=false to write to disk.' + : `Plan created at ${planDir}. Use provar.testplan.create-suite to add suites, then provar.testplan.add-instance to wire test cases into the plan.`, + }; + return { + content: [{ type: 'text' as const, text: JSON.stringify(response) }], + structuredContent: response, + }; + } catch (err: unknown) { + const error = err as Error & { code?: string }; + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify( + makeError( + error instanceof PathPolicyError ? error.code : (error.code ?? 'CREATE_PLAN_ERROR'), + error.message, + requestId + ) + ), + }, + ], + }; + } + } + ); +} + // ── provar.testplan.add-instance ────────────────────────────────────────────── export function registerTestPlanAddInstance(server: McpServer, config: ServerConfig): void { @@ -358,6 +484,7 @@ export function registerTestPlanRemoveInstance(server: McpServer, config: Server // ── Convenience re-export ───────────────────────────────────────────────────── export function registerAllTestPlanTools(server: McpServer, config: ServerConfig): void { + registerTestPlanCreate(server, config); registerTestPlanAddInstance(server, config); registerTestPlanCreateSuite(server, config); registerTestPlanRemoveInstance(server, config); diff --git a/test/unit/mcp/testCaseGenerate.test.ts b/test/unit/mcp/testCaseGenerate.test.ts index bbdcf1b..e3729be 100644 --- a/test/unit/mcp/testCaseGenerate.test.ts +++ b/test/unit/mcp/testCaseGenerate.test.ts @@ -460,6 +460,250 @@ describe('provar.testcase.generate', () => { }); }); + describe('D2 — uiTarget / uiLocator argument types', () => { + it('emits class="uiTarget" uri="..." for the target argument', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'UI Target Test', + steps: [ + { + api_id: 'UiWithScreen', + name: 'With page', + attributes: { target: 'sf:ui:target?pageObject=pageobjects.Account&flexiPage=Account_flexiPage' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="uiTarget"'), 'Expected class="uiTarget"'); + assert.ok(xml.includes('uri="sf:ui:target?'), 'Expected uri attribute with sf:ui:target value'); + assert.ok(!xml.includes('valueClass="string">sf:ui:target'), 'Must NOT emit uiTarget URI as a plain string value'); + }); + + it('emits class="uiLocator" uri="..." for the locator argument', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'UI Locator Test', + steps: [ + { + api_id: 'UiDoAction', + name: 'Click button', + attributes: { locator: 'sf:ui:locator:button?label=Save' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="uiLocator"'), 'Expected class="uiLocator"'); + assert.ok(xml.includes('uri="sf:ui:locator:'), 'Expected uri attribute with locator value'); + assert.ok(!xml.includes('valueClass="string">sf:ui:locator'), 'Must NOT emit locator URI as a plain string value'); + }); + + it('uiTarget also applies inside UiWithScreen wrapper when target_uri is non-SF', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Non-SF With Target', + steps: [], + target_uri: 'ui:pageobject:target?pageId=pageobjects.LoginPage', + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="uiTarget"'), 'Wrapper UiWithScreen target should use uiTarget class'); + assert.ok(xml.includes('uri="ui:pageobject:target?pageId=pageobjects.LoginPage"'), 'URI should appear as attribute'); + }); + }); + + describe('D3 — SetValues / AssertValues use valueList/namedValues structure', () => { + it('SetValues emits with ', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'SetValues Test', + steps: [ + { + api_id: 'SetValues', + name: 'Set test vars', + attributes: { testCaseName: 'TC_New', testType: 'Acceptance testing' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="valueList"'), 'Expected class="valueList"'); + assert.ok(xml.includes('mutable="Mutable"'), 'Expected mutable="Mutable"'); + assert.ok(xml.includes(''), 'Expected element'); + assert.ok(xml.includes(''), 'Expected namedValue for testCaseName'); + assert.ok(xml.includes(''), 'Expected namedValue for testType'); + assert.ok(xml.includes(''), 'Expected argument id="values"'); + assert.ok( + !xml.includes('testCaseName|TC_New'), + 'Must NOT emit pipe-delimited string for SetValues' + ); + }); + + it('AssertValues uses flat argument structure (not valueList)', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'AssertValues Test', + steps: [ + { + api_id: 'AssertValues', + name: 'Assert vars', + attributes: { opportunityName: 'My Opp' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes(''), 'Expected flat argument id for AssertValues'); + assert.ok(!xml.includes('class="valueList"'), 'AssertValues must NOT emit valueList structure'); + assert.ok(!xml.includes(' { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Flat Args Test', + steps: [ + { api_id: 'ApexCreateObject', name: 'Create record', attributes: { objectApiName: 'Opportunity' } }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes(''), 'Expected flat argument id'); + assert.ok(!xml.includes('valueList'), 'Must NOT emit valueList for non-SetValues steps'); + }); + }); + + describe('D4 — Variable references use class="variable" with elements', () => { + it('{VarName} emits class="variable" ', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Variable Ref Test', + steps: [ + { + api_id: 'ApexCreateObject', + name: 'Create record', + attributes: { provar__Test_Project__c: '{TestProjectId}' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="variable"'), 'Expected class="variable"'); + assert.ok(xml.includes(''), 'Expected '); + assert.ok(!xml.includes('valueClass="string">{TestProjectId}'), 'Must NOT emit {VarName} as a string literal'); + }); + + it('{Obj.Field} dotted path emits two elements', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Dotted Variable Test', + steps: [ + { + api_id: 'ApexCreateObject', + name: 'Create with nested var', + attributes: { Name: '{Opportunity.Name}' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes(''), 'Expected first path element'); + assert.ok(xml.includes(''), 'Expected second path element'); + }); + + it('variable reference also works inside SetValues namedValues', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'SetValues With Var', + steps: [ + { + api_id: 'SetValues', + name: 'Set with variable', + attributes: { projectId: '{TestProjectId}', label: 'Static Label' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="variable"'), 'Expected variable reference inside namedValues'); + assert.ok(xml.includes('')); + assert.ok(xml.includes('valueClass="string">Static Label'), 'Static value should still be a plain string'); + }); + + it('plain string values without braces are not treated as variable references', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'No Var Test', + steps: [{ api_id: 'ApexCreateObject', name: 'Create', attributes: { Name: 'Literal Name' } }], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('valueClass="string">Literal Name'), 'Plain string should use valueClass="string"'); + assert.ok(!xml.includes('class="variable"'), 'No variable element expected'); + }); + }); + + describe('D7 — Cleanup warning for ApexDeleteObject', () => { + it('includes cleanup warning when ApexDeleteObject is in the step list', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Create and Delete', + steps: [ + { api_id: 'ApexCreateObject', name: 'Create record', attributes: { objectApiName: 'Account' } }, + { api_id: 'ApexDeleteObject', name: 'Delete record', attributes: { objectApiName: 'Account' } }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const warnings = body['warnings'] as string[] | undefined; + assert.ok(Array.isArray(warnings) && warnings.length > 0, 'Expected at least one warning'); + assert.ok( + warnings.some((w) => w.includes('ApexDeleteObject') && w.includes('cleanup')), + 'Expected cleanup warning mentioning ApexDeleteObject' + ); + }); + + it('does NOT warn when no ApexDeleteObject steps are present', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'No Cleanup', + steps: [{ api_id: 'ApexCreateObject', name: 'Create', attributes: {} }], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const warnings = body['warnings'] as string[] | undefined; + const hasCleanupWarning = warnings?.some((w) => w.includes('ApexDeleteObject')); + assert.ok(!hasCleanupWarning, 'No cleanup warning expected without ApexDeleteObject'); + }); + }); + describe('validate_after_edit', () => { it('includes validation field when validate_after_edit=true (default)', () => { const result = server.call('provar.testcase.generate', { diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index 628ce59..456fc09 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -293,6 +293,347 @@ describe('validateTestCase', () => { assert.ok(!r.issues.some((i) => i.rule_id === 'ASSERT-001'), 'ASSERT-001 should not fire for SetValues'); }); }); + + describe('UI-TARGET-001', () => { + it('errors when UiWithScreen target argument uses class="value" (plain string)', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:target?object=Account + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-TARGET-001'), + 'Expected UI-TARGET-001' + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-TARGET-001')!; + assert.equal(issue.severity, 'ERROR'); + assert.ok(issue.message.includes('uiTarget'), `Message should mention uiTarget: ${issue.message}`); + }); + + it('does not fire when UiWithScreen target uses class="uiTarget"', () => { + const r = validateTestCase( + ` + + + + + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'UI-TARGET-001'), + 'UI-TARGET-001 should not fire for correct uiTarget class' + ); + }); + + it('does not fire when UiWithScreen has no target argument', () => { + const r = validateTestCase( + ` + + + + + 1280 + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'UI-TARGET-001'), + 'UI-TARGET-001 should not fire when no target argument present' + ); + }); + + it('also fires for UiWithRow steps with wrong target class', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:target?object=Account + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-TARGET-001'), + 'Expected UI-TARGET-001 for UiWithRow' + ); + }); + }); + + describe('UI-LOCATOR-001', () => { + it('errors when UiDoAction locator argument uses class="value" (plain string)', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:locator:label?label=Save + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-LOCATOR-001'), + 'Expected UI-LOCATOR-001' + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-LOCATOR-001')!; + assert.equal(issue.severity, 'ERROR'); + assert.ok(issue.message.includes('uiLocator'), `Message should mention uiLocator: ${issue.message}`); + }); + + it('does not fire when UiDoAction locator uses class="uiLocator"', () => { + const r = validateTestCase( + ` + + + + + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'UI-LOCATOR-001'), + 'UI-LOCATOR-001 should not fire for correct uiLocator class' + ); + }); + + it('also fires for UiAssert steps with wrong locator class', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:locator:label?label=Name + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-LOCATOR-001'), + 'Expected UI-LOCATOR-001 for UiAssert' + ); + }); + }); + + describe('SETVALUES-STRUCTURE-001', () => { + it('errors when SetValues values argument uses class="value" (plain string)', () => { + const r = validateTestCase( + ` + + + + + + myVar=hello + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'SETVALUES-STRUCTURE-001'), + 'Expected SETVALUES-STRUCTURE-001' + ); + const issue = r.issues.find((i) => i.rule_id === 'SETVALUES-STRUCTURE-001')!; + assert.equal(issue.severity, 'ERROR'); + assert.ok(issue.message.includes('valueList'), `Message should mention valueList: ${issue.message}`); + }); + + it('does not fire when SetValues values argument uses class="valueList"', () => { + const r = validateTestCase( + ` + + + + + + + + + hello + + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'SETVALUES-STRUCTURE-001'), + 'SETVALUES-STRUCTURE-001 should not fire for correct valueList structure' + ); + }); + + it('does not fire when SetValues has no values argument (self-closing)', () => { + const r = validateTestCase( + ` + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'SETVALUES-STRUCTURE-001'), + 'SETVALUES-STRUCTURE-001 should not fire when no values argument present' + ); + }); + + it('does not fire for AssertValues (only targets SetValues)', () => { + const r = validateTestCase( + ` + + + + + + myVar=hello + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'SETVALUES-STRUCTURE-001'), + 'SETVALUES-STRUCTURE-001 should not fire for AssertValues' + ); + }); + }); + + describe('VAR-REF-001', () => { + it('warns when a plain string value contains a {VarName} pattern', () => { + const r = validateTestCase( + ` + + + + + + {AccountId} + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'VAR-REF-001'), + 'Expected VAR-REF-001' + ); + const issue = r.issues.find((i) => i.rule_id === 'VAR-REF-001')!; + assert.equal(issue.severity, 'WARNING'); + assert.ok(issue.message.includes('{AccountId}'), `Message should include variable name: ${issue.message}`); + }); + + it('warns for dotted path {Obj.Field} stored as plain string', () => { + const r = validateTestCase( + ` + + + + + + {Record.Name} + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'VAR-REF-001'), + 'Expected VAR-REF-001 for dotted path' + ); + assert.ok( + r.issues.find((i) => i.rule_id === 'VAR-REF-001')!.message.includes('{Record.Name}'), + 'Message should include dotted variable name' + ); + }); + + it('does not fire when value uses class="variable" (correct structure)', () => { + const r = validateTestCase( + ` + + + + + + + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'VAR-REF-001'), + 'VAR-REF-001 should not fire for correct class="variable" structure' + ); + }); + + it('does not fire for plain string content without curly braces', () => { + const r = validateTestCase( + ` + + + + + + John Smith + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'VAR-REF-001'), + 'VAR-REF-001 should not fire for plain string without curly braces' + ); + }); + }); }); // ── Handler-level tests (registerTestCaseValidate) ──────────────────────────── diff --git a/test/unit/mcp/testPlanTools.test.ts b/test/unit/mcp/testPlanTools.test.ts index e3d7184..28f299f 100644 --- a/test/unit/mcp/testPlanTools.test.ts +++ b/test/unit/mcp/testPlanTools.test.ts @@ -96,6 +96,135 @@ afterEach(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); +// ── provar.testplan.create ──────────────────────────────────────────────────── + +describe('provar.testplan.create', () => { + describe('happy path', () => { + it('creates plan directory and .planitem, returns expected fields', () => { + makeProject(projectDir); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'MyNewPlan', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['created'], true); + assert.equal(body['dry_run'], false); + assert.ok(typeof body['guid'] === 'string' && body['guid'].length > 0, 'Expected guid'); + + const planDir = path.join(projectDir, 'plans', 'MyNewPlan'); + assert.ok(fs.existsSync(planDir), 'Plan directory should be created'); + assert.ok(fs.existsSync(path.join(planDir, '.planitem')), '.planitem should be written'); + + const xml = fs.readFileSync(path.join(planDir, '.planitem'), 'utf-8'); + assert.ok(xml.includes(' { + makeProject(projectDir); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'GuidedPlan', + overwrite: false, + dry_run: false, + }); + + const body = parseText(result); + assert.ok(typeof body['next_steps'] === 'string' && body['next_steps'].length > 0, 'Expected next_steps'); + }); + }); + + describe('dry_run', () => { + it('returns created=false and does not write to disk', () => { + makeProject(projectDir); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'DryPlan', + overwrite: false, + dry_run: true, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['created'], false); + assert.equal(body['dry_run'], true); + assert.ok(typeof body['guid'] === 'string', 'Expected guid even in dry_run'); + + const planDir = path.join(projectDir, 'plans', 'DryPlan'); + assert.equal(fs.existsSync(planDir), false, 'Directory must not be created in dry_run mode'); + }); + }); + + describe('error cases', () => { + it('returns NOT_A_PROJECT when .testproject is missing', () => { + fs.mkdirSync(projectDir, { recursive: true }); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'MyPlan', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), true); + assert.equal(errorCode(result), 'NOT_A_PROJECT'); + }); + + it('returns PLAN_EXISTS when .planitem already exists and overwrite=false', () => { + makeProject(projectDir); + makePlan(projectDir, 'ExistingPlan'); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'ExistingPlan', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), true); + assert.equal(errorCode(result), 'PLAN_EXISTS'); + }); + + it('overwrites .planitem when overwrite=true and plan already exists', () => { + makeProject(projectDir); + makePlan(projectDir, 'ExistingPlan'); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'ExistingPlan', + overwrite: true, + dry_run: false, + }); + + assert.equal(isError(result), false); + assert.equal(parseText(result)['created'], true); + }); + + it('returns PATH_NOT_ALLOWED when project_path is outside allowedPaths', () => { + const strictServer = new MockMcpServer(); + registerAllTestPlanTools(strictServer as never, { allowedPaths: [tmpDir] }); + + const result = strictServer.call('provar.testplan.create', { + project_path: path.join(os.tmpdir(), 'outside-project'), + plan_name: 'MyPlan', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), true); + const code = errorCode(result); + assert.ok(code === 'PATH_NOT_ALLOWED' || code === 'PATH_TRAVERSAL', `Unexpected error code: ${code}`); + }); + }); +}); + // ── provar.testplan.add-instance ─────────────────────────────────────────────── describe('provar.testplan.add-instance', () => { @@ -585,14 +714,20 @@ describe('provar.testplan.remove-instance', () => { // ── registerAllTestPlanTools ─────────────────────────────────────────────────── describe('registerAllTestPlanTools', () => { - it('registers all three tools', () => { + it('registers all four tools', () => { const freshServer = new MockMcpServer(); registerAllTestPlanTools(freshServer as never, config); - // Each tool should be callable without throwing "Tool not registered" makeProject(projectDir); - // Verify each tool is registered by checking it doesn't throw + assert.doesNotThrow(() => { + freshServer.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'P', + overwrite: false, + dry_run: true, + }); + }); assert.doesNotThrow(() => { freshServer.call('provar.testplan.add-instance', { project_path: projectDir,