From 37c54ff32ca263847f2e5e2e9735d0417745b4fa Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Thu, 4 Jun 2026 15:28:14 -0500 Subject: [PATCH] =?UTF-8?q?PDX-0:=20fix(mcp):=20TC=5F010=20=E2=80=94=20acc?= =?UTF-8?q?ept=20any=20integer=20testCase=20id=20and=20make=20id=20optiona?= =?UTF-8?q?l?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RCA: TC_010 required the testCase id attribute to be the literal "1" ("Provar requires id=1"), but that belief is wrong. Test case ids are project-unique integers numbered sequentially, and the guid — not id — is the cross-project unique identifier. A full AllPOCProjects scan shows real, loadable test cases with ids 0..51 and many with no id attribute at all. The strict rule therefore fired TC_010 (an is_valid-gating ERROR) on ~28 of 29 real files — a false positive that wrongly marked valid test cases invalid. Fix: Make id optional (missing id is no longer an error — guid identifies the test case) and, when present, require only a non-negative integer; a present-but -non-numeric id (e.g. a UUID) is still rejected. Verified against the corpus: TC_010 now fires on 0 of 123 files (previously ~28/29). Tests updated to cover missing id, id="0", id="42", and a UUID-as-id. --- src/mcp/tools/testCaseValidate.ts | 19 +++++++---------- test/unit/mcp/testCaseValidate.test.ts | 29 ++++++++++++++++++++------ 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index c103566..15f2b37 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/src/mcp/tools/testCaseValidate.ts @@ -384,21 +384,18 @@ export function validateTestCaseXml(filePath: string, config: ServerConfig): Tes /** TC_010/TC_011: validate testCase id and guid attributes. */ function checkTestCaseIdAndGuid(tcId: string | null, tcGuid: string | undefined, issues: ValidationIssue[]): void { - if (!tcId) { + // The testCase `id` is OPTIONAL — the guid is the unique identifier (real Provar + // projects routinely omit id). When an id IS present it must be a non-negative + // integer: test case ids are project-unique sequential numbers (corpus shows 0…N), + // NOT the literal "1". Only a present-but-non-numeric id (e.g. a UUID) is rejected. + if (tcId !== null && !/^\d+$/.test(tcId)) { issues.push({ rule_id: 'TC_010', severity: 'ERROR', - message: 'testCase missing required id attribute.', + message: `testCase id="${tcId}" is invalid — when present, the id must be a non-negative integer (test case ids are unique within a project, numbered sequentially). The guid is the cross-project unique identifier.`, applies_to: 'testCase', - suggestion: 'Add id="1" to testCase element (Provar requires the integer literal "1").', - }); - } else if (tcId !== '1') { - issues.push({ - rule_id: 'TC_010', - severity: 'ERROR', - message: `testCase id="${tcId}" is invalid — Provar requires id="1" (integer literal).`, - applies_to: 'testCase', - suggestion: 'Set id="1" on the testCase element. The unique identifier is the guid attribute, not id.', + suggestion: + 'Set id to a project-unique non-negative integer (typically the highest in use + 1), or omit the id attribute entirely and rely on the guid.', }); } if (!tcGuid) { diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index 6adc8b7..c3ca6d6 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -77,31 +77,48 @@ describe('validateTestCase', () => { }); describe('testCase attribute rules', () => { - it('TC_010: flags missing id', () => { + it('TC_010: does NOT fire for a missing id — id is optional, guid is the identifier', () => { const r = validateTestCase( `` ); assert.ok( - r.issues.some((i) => i.rule_id === 'TC_010'), - 'Expected TC_010' + !r.issues.some((i) => i.rule_id === 'TC_010'), + 'A missing id must not be flagged (real Provar test cases routinely omit it)' ); }); - it('TC_010: flags non-"1" id (e.g. UUID used as id)', () => { + it('TC_010: flags a non-numeric id (e.g. UUID used as id)', () => { const r = validateTestCase( `` ); assert.ok( r.issues.some((i) => i.rule_id === 'TC_010'), - 'Expected TC_010 for UUID used as id' + 'Expected TC_010 for a non-numeric id' + ); + }); + + it('TC_010: does not fire for id="0" — non-negative integers are valid (corpus uses 0)', () => { + const r = validateTestCase( + `` ); + assert.ok(!r.issues.some((i) => i.rule_id === 'TC_010'), 'id="0" must be accepted'); }); - it('TC_010: does not fire when id="1" (the correct literal)', () => { + it('TC_010: does not fire for a valid integer id (id="1")', () => { const r = validateTestCase(VALID_TC); assert.ok(!r.issues.some((i) => i.rule_id === 'TC_010'), 'TC_010 must not fire when id="1"'); }); + it('TC_010: does not fire for a sequential project id (e.g. id="42") — ids are not literally "1"', () => { + const r = validateTestCase( + `` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'TC_010'), + 'TC_010 must accept any non-negative-integer id (test case ids are project-unique sequential numbers)' + ); + }); + it('TC_011: flags missing guid', () => { const r = validateTestCase( ''