From 86df41ea51284eab04fd383e90c2207e63b43548 Mon Sep 17 00:00:00 2001 From: Ramil Amparo Date: Wed, 1 Apr 2026 14:09:09 +0400 Subject: [PATCH 1/6] Fix incorrect Playwright upload mapping to QA Sphere runs Three bugs caused Playwright test results to map to wrong QA Sphere test cases, producing misleading run summaries: 1. Marker matching used substring includes, so QS1-104 incorrectly matched QS1-10427. Replace with word-boundary regex. 2. Playwright parser only used the first "test case" annotation per test, dropping additional ones. Now fans out one TestCaseResult per annotation. 3. Upload message said "Uploaded N test cases" conflating result count with target test case count. Now reports both: "Uploaded X results to Y test cases". Additionally, deduplicate file uploads when multiple results reference the same attachment. Previously each result uploaded its own copy. Now uploads each unique file once and reuses the URL. Progress message shows duplicates skipped count. Closes Hypersequent/tms-issues#2394 --- .../allure/matching-tcases/005-result.json | 2 +- .../missing-attachments/004-result.json | 2 +- .../fixtures/junit-xml/matching-tcases.xml | 2 +- .../junit-xml/missing-attachments.xml | 2 +- .../playwright-json/matching-tcases.json | 2 +- .../playwright-json/missing-attachments.json | 2 +- .../multi-annotation-with-attachments.json | 81 +++++++++++ src/tests/fixtures/testcases.ts | 34 +++++ src/tests/marker-parser.spec.ts | 34 +++++ src/tests/playwright-json-parsing.spec.ts | 133 ++++++++++++++++++ src/tests/result-upload.spec.ts | 67 ++++++++- src/utils/result-upload/MarkerParser.ts | 5 +- src/utils/result-upload/ResultUploader.ts | 51 ++++--- .../parsers/playwrightJsonParser.ts | 57 ++++---- src/utils/result-upload/types.ts | 1 + src/utils/result-upload/utils.ts | 3 +- 16 files changed, 421 insertions(+), 57 deletions(-) create mode 100644 src/tests/fixtures/playwright-json/multi-annotation-with-attachments.json diff --git a/src/tests/fixtures/allure/matching-tcases/005-result.json b/src/tests/fixtures/allure/matching-tcases/005-result.json index 16f04ab..8698bb2 100644 --- a/src/tests/fixtures/allure/matching-tcases/005-result.json +++ b/src/tests/fixtures/allure/matching-tcases/005-result.json @@ -10,7 +10,7 @@ "attachments": [ { "name": "attachment", - "source": "shared-attachment.txt", + "source": "006-container.json", "type": "text/plain" } ], diff --git a/src/tests/fixtures/allure/missing-attachments/004-result.json b/src/tests/fixtures/allure/missing-attachments/004-result.json index 68b92c2..5f08586 100644 --- a/src/tests/fixtures/allure/missing-attachments/004-result.json +++ b/src/tests/fixtures/allure/missing-attachments/004-result.json @@ -10,7 +10,7 @@ "attachments": [ { "name": "attachment", - "source": "existing-attachment.txt", + "source": "001-result.json", "type": "text/plain" } ] diff --git a/src/tests/fixtures/junit-xml/matching-tcases.xml b/src/tests/fixtures/junit-xml/matching-tcases.xml index fda43a5..e66c4c2 100644 --- a/src/tests/fixtures/junit-xml/matching-tcases.xml +++ b/src/tests/fixtures/junit-xml/matching-tcases.xml @@ -173,7 +173,7 @@ diff --git a/src/tests/fixtures/junit-xml/missing-attachments.xml b/src/tests/fixtures/junit-xml/missing-attachments.xml index 2df1b69..1825ecc 100644 --- a/src/tests/fixtures/junit-xml/missing-attachments.xml +++ b/src/tests/fixtures/junit-xml/missing-attachments.xml @@ -173,7 +173,7 @@ diff --git a/src/tests/fixtures/playwright-json/matching-tcases.json b/src/tests/fixtures/playwright-json/matching-tcases.json index 96728e2..1fefbe2 100644 --- a/src/tests/fixtures/playwright-json/matching-tcases.json +++ b/src/tests/fixtures/playwright-json/matching-tcases.json @@ -149,7 +149,7 @@ { "name": "attachment", "contentType": "application/json", - "path": "./matching-tcases.json" + "path": "./empty-tsuite.json" } ] } diff --git a/src/tests/fixtures/playwright-json/missing-attachments.json b/src/tests/fixtures/playwright-json/missing-attachments.json index 48d85ca..6517230 100644 --- a/src/tests/fixtures/playwright-json/missing-attachments.json +++ b/src/tests/fixtures/playwright-json/missing-attachments.json @@ -149,7 +149,7 @@ { "name": "attachment", "contentType": "application/json", - "path": "./matching-tcases.json" + "path": "./empty-tsuite.json" } ] } diff --git a/src/tests/fixtures/playwright-json/multi-annotation-with-attachments.json b/src/tests/fixtures/playwright-json/multi-annotation-with-attachments.json new file mode 100644 index 0000000..a6ae8cc --- /dev/null +++ b/src/tests/fixtures/playwright-json/multi-annotation-with-attachments.json @@ -0,0 +1,81 @@ +{ + "suites": [ + { + "title": "multi-annotation.spec.ts", + "specs": [ + { + "title": "Login flow covers multiple cases", + "tags": [], + "tests": [ + { + "annotations": [ + { + "type": "test case", + "description": "https://qas.eu1.qasphere.com/project/TEST/tcase/10427" + }, + { + "type": "test case", + "description": "https://qas.eu1.qasphere.com/project/TEST/tcase/10427" + }, + { + "type": "test case", + "description": "https://qas.eu1.qasphere.com/project/TEST/tcase/10428" + } + ], + "expectedStatus": "passed", + "projectName": "chromium", + "results": [ + { + "status": "passed", + "errors": [], + "stdout": [], + "stderr": [], + "retry": 0, + "duration": 2500, + "attachments": [ + { + "name": "attachment", + "contentType": "application/json", + "path": "./multi-annotation-with-attachments.json" + } + ] + } + ], + "status": "expected" + } + ] + }, + { + "title": "Navigation bar items TEST-006", + "tags": [], + "tests": [ + { + "annotations": [], + "expectedStatus": "passed", + "projectName": "chromium", + "results": [ + { + "status": "passed", + "errors": [], + "stdout": [], + "stderr": [], + "retry": 0, + "duration": 1000, + "attachments": [ + { + "name": "attachment", + "contentType": "application/json", + "path": "./multi-annotation-with-attachments.json" + } + ] + } + ], + "status": "expected" + } + ] + } + ], + "suites": [] + } + ] +} diff --git a/src/tests/fixtures/testcases.ts b/src/tests/fixtures/testcases.ts index be3c773..71f019d 100644 --- a/src/tests/fixtures/testcases.ts +++ b/src/tests/fixtures/testcases.ts @@ -118,4 +118,38 @@ export const runTestCases = [ pos: 1, }, }, + { + id: '1CBd7Qsn1_abc10427xyz', + version: 1, + folderId: 13, + pos: 4, + seq: 10427, + title: 'Login flow case 1', + priority: 'medium', + status: 'open', + folder: { + id: 13, + parentId: 7, + title: 'multi-annotation.spec.ts', + projectId: '1CBd7PKmG_khB9xTm8gL2oe', + pos: 1, + }, + }, + { + id: '1CBd7Qsn2_abc10428xyz', + version: 1, + folderId: 13, + pos: 5, + seq: 10428, + title: 'Login flow case 2', + priority: 'medium', + status: 'open', + folder: { + id: 13, + parentId: 7, + title: 'multi-annotation.spec.ts', + projectId: '1CBd7PKmG_khB9xTm8gL2oe', + pos: 1, + }, + }, ] diff --git a/src/tests/marker-parser.spec.ts b/src/tests/marker-parser.spec.ts index 0d43317..c440dce 100644 --- a/src/tests/marker-parser.spec.ts +++ b/src/tests/marker-parser.spec.ts @@ -216,6 +216,40 @@ describe('nameMatchesTCase', () => { test('no match for wrong seq', () => { expect(junit.nameMatchesTCase('TEST-002 Cart', 'TEST', 3)).toBe(false) }) + + describe('does not prefix-match longer sequences', () => { + test('QS1-104 does not match name containing QS1-10427', () => { + expect(playwright.nameMatchesTCase('QS1-10427: some test', 'QS1', 104)).toBe(false) + }) + + test('QS1-107 does not match name containing QS1-10775', () => { + expect(playwright.nameMatchesTCase('QS1-10775: some test', 'QS1', 107)).toBe(false) + }) + + test('QS1-10427 still matches itself', () => { + expect(playwright.nameMatchesTCase('QS1-10427: some test', 'QS1', 10427)).toBe(true) + }) + + test('QS1-104 matches when it appears exactly', () => { + expect(playwright.nameMatchesTCase('QS1-104: some test', 'QS1', 104)).toBe(true) + }) + + test('QS1-104 matches at end of name', () => { + expect(playwright.nameMatchesTCase('some test QS1-104', 'QS1', 104)).toBe(true) + }) + + test('QS1-104 matches in middle of name with boundaries', () => { + expect(playwright.nameMatchesTCase('some QS1-104 test', 'QS1', 104)).toBe(true) + }) + + test('marker surrounded by parens still matches', () => { + expect(playwright.nameMatchesTCase('some test (QS1-10427)', 'QS1', 10427)).toBe(true) + }) + + test('marker surrounded by parens does not prefix-match', () => { + expect(playwright.nameMatchesTCase('some test (QS1-10427)', 'QS1', 104)).toBe(false) + }) + }) }) describe('separator-bounded hyphenless (JUnit only)', () => { diff --git a/src/tests/playwright-json-parsing.spec.ts b/src/tests/playwright-json-parsing.spec.ts index ced352f..630f0f0 100644 --- a/src/tests/playwright-json-parsing.spec.ts +++ b/src/tests/playwright-json-parsing.spec.ts @@ -379,6 +379,139 @@ describe('Playwright JSON parsing', () => { expect(testcases[2].name).toBe('PRJ-789: PRJ-456: Test with marker in name and annotation') }) + test('Should fan out multiple results for test with multiple annotations', async () => { + const jsonPath = `${playwrightJsonBasePath}/multi-annotation-with-attachments.json` + const jsonContent = await readFile(jsonPath, 'utf8') + + const { testCaseResults: testcases } = await parsePlaywrightJson(jsonContent, '', { + skipStdout: 'never', + skipStderr: 'never', + }) + + // Fixture has 1 test with 3 annotations (2 for 10427, 1 for 10428) + 1 test with no annotations = 4 results + expect(testcases).toHaveLength(4) + expect(testcases[0].name).toBe('TEST-10427: Login flow covers multiple cases') + expect(testcases[1].name).toBe('TEST-10427: Login flow covers multiple cases') + expect(testcases[2].name).toBe('TEST-10428: Login flow covers multiple cases') + expect(testcases[3].name).toBe('Navigation bar items TEST-006') + + // The three fan-out entries share the same status, duration, folder + for (const tc of testcases.slice(0, 3)) { + expect(tc.status).toBe('passed') + expect(tc.timeTaken).toBe(2500) + expect(tc.folder).toBe('multi-annotation.spec.ts') + } + + // All four entries have attachments from the fixture + for (const tc of testcases) { + expect(tc.attachments).toHaveLength(1) + } + }) + + test('Should still produce one result for single annotation', async () => { + const jsonContent = JSON.stringify({ + suites: [ + { + title: 'single.spec.ts', + specs: [ + { + title: 'Simple test', + tags: [], + tests: [ + { + annotations: [ + { + type: 'test case', + description: 'https://qas.eu1.qasphere.com/project/PRJ/tcase/100', + }, + ], + expectedStatus: 'passed', + projectName: 'chromium', + results: [ + { + status: 'passed', + errors: [], + stdout: [], + stderr: [], + retry: 0, + duration: 1000, + attachments: [], + }, + ], + status: 'expected', + }, + ], + }, + ], + suites: [], + }, + ], + }) + + const { testCaseResults: testcases } = await parsePlaywrightJson(jsonContent, '', { + skipStdout: 'never', + skipStderr: 'never', + }) + + expect(testcases).toHaveLength(1) + expect(testcases[0].name).toBe('PRJ-100: Simple test') + }) + + test('Should fan out by annotations even when name has a marker', async () => { + const jsonContent = JSON.stringify({ + suites: [ + { + title: 'precedence.spec.ts', + specs: [ + { + title: 'PRJ-999: Test with marker in name', + tags: [], + tests: [ + { + annotations: [ + { + type: 'test case', + description: 'https://qas.eu1.qasphere.com/project/PRJ/tcase/100', + }, + { + type: 'test case', + description: 'https://qas.eu1.qasphere.com/project/PRJ/tcase/200', + }, + ], + expectedStatus: 'passed', + projectName: 'chromium', + results: [ + { + status: 'passed', + errors: [], + stdout: [], + stderr: [], + retry: 0, + duration: 1000, + attachments: [], + }, + ], + status: 'expected', + }, + ], + }, + ], + suites: [], + }, + ], + }) + + const { testCaseResults: testcases } = await parsePlaywrightJson(jsonContent, '', { + skipStdout: 'never', + skipStderr: 'never', + }) + + // Annotations take precedence — two results, not one from the name marker + expect(testcases).toHaveLength(2) + expect(testcases[0].name).toBe('PRJ-100: PRJ-999: Test with marker in name') + expect(testcases[1].name).toBe('PRJ-200: PRJ-999: Test with marker in name') + }) + test('Should map test status correctly', async () => { const jsonContent = JSON.stringify({ suites: [ diff --git a/src/tests/result-upload.spec.ts b/src/tests/result-upload.spec.ts index 548ef27..31570d7 100644 --- a/src/tests/result-upload.spec.ts +++ b/src/tests/result-upload.spec.ts @@ -1,7 +1,7 @@ import { HttpResponse, http } from 'msw' import { setupServer } from 'msw/node' import { unlinkSync, readdirSync } from 'node:fs' -import { afterAll, beforeAll, beforeEach, expect, test, describe, afterEach } from 'vitest' +import { afterAll, beforeAll, beforeEach, expect, test, describe, afterEach, vi } from 'vitest' import { run } from '../commands/main' import { CreateTCasesRequest, @@ -156,6 +156,16 @@ const countCreateTCasesApiCalls = () => const countRunLogApiCalls = () => countMockedApiCalls(server, (req) => new URL(req.url).pathname.endsWith(`/run/${runId}/log`)) +const countIndividualFileUploads = () => { + let count = 0 + server.events.on('response:mocked', async (e) => { + if (!new URL(e.request.url).pathname.endsWith('/file/batch')) return + const body = (await e.response.clone().json()) as { files: unknown[] } + count += body.files.length + }) + return () => count +} + const getMappingFiles = () => new Set( readdirSync('.').filter((f) => f.startsWith('qasphere-automapping-') && f.endsWith('.txt')) @@ -337,6 +347,17 @@ fileTypesWithAllure.forEach((fileType) => { expect(numResultUploadCalls()).toBe(3) // 5 results total }) + test('Upload success message should report results and unique test cases', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + await run( + `${fileType.command} -r ${runURL} ${fixtureInputPath(fileType, 'matching-tcases')}` + ) + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('Uploaded 5 results to 5 test cases') + ) + logSpy.mockRestore() + }) + test('Test cases on reports with a missing test case on QAS should throw an error', async () => { const numFileUploadCalls = countFileUploadApiCalls() const numResultUploadCalls = countResultUploadApiCalls() @@ -391,14 +412,18 @@ fileTypesWithAllure.forEach((fileType) => { }) describe('Uploading with attachments', () => { - test('Attachments should be uploaded', async () => { + test('Attachments should be uploaded and deduplicated', async () => { const numFileUploadCalls = countFileUploadApiCalls() const numResultUploadCalls = countResultUploadApiCalls() + const totalFilesUploaded = countIndividualFileUploads() setMaxResultsInRequest(3) await run( `${fileType.command} -r ${runURL} --attachments ${fixtureInputPath(fileType, 'matching-tcases')}` ) - expect(numFileUploadCalls()).toBe(1) // all 5 files in one batch + // matching-tcases fixtures have 4 tests with the same attachment + 1 unique + // Should upload 2 unique files, not 5 + expect(numFileUploadCalls()).toBe(1) + expect(totalFilesUploaded()).toBe(2) expect(numResultUploadCalls()).toBe(2) // 5 results total }) test('Missing attachments should throw an error', async () => { @@ -415,11 +440,15 @@ fileTypesWithAllure.forEach((fileType) => { test('Missing attachments should be successful when forced', async () => { const numFileUploadCalls = countFileUploadApiCalls() const numResultUploadCalls = countResultUploadApiCalls() + const totalFilesUploaded = countIndividualFileUploads() setMaxResultsInRequest(1) await run( `${fileType.command} -r ${runURL} --attachments --force ${fixtureInputPath(fileType, 'missing-attachments')}` ) - expect(numFileUploadCalls()).toBe(1) // all 4 files in one batch + // missing-attachments fixtures have 3 tests with the same attachment + 1 unique + 1 missing + // Should upload 2 unique valid files, not 4 + expect(numFileUploadCalls()).toBe(1) + expect(totalFilesUploaded()).toBe(2) expect(numResultUploadCalls()).toBe(5) // 5 results total }) }) @@ -615,6 +644,36 @@ describe('Allure invalid result file handling', () => { }) }) +describe('Multi-annotation Playwright upload', () => { + test('Should deduplicate file uploads when fan-out entries share the same attachment', async () => { + const totalFilesUploaded = countIndividualFileUploads() + + await run( + `playwright-json-upload -r ${runURL} --attachments ./src/tests/fixtures/playwright-json/multi-annotation-with-attachments.json` + ) + + // The fixture has 1 test with 3 annotations (fan-out to 3 results) and 1 test with no + // annotations. All 4 results reference the same attachment file. + // Only 1 unique file should be uploaded, not 4. + expect(totalFilesUploaded()).toBe(1) + }) + + test('Upload message should distinguish results from unique test cases', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + + await run( + `playwright-json-upload -r ${runURL} ./src/tests/fixtures/playwright-json/multi-annotation-with-attachments.json` + ) + + // Fixture: 3 fan-out results (2x TEST-10427, 1x TEST-10428) + 1 regular (TEST-006) + // = 4 results mapping to 3 unique test cases + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('Uploaded 4 results to 3 test cases') + ) + logSpy.mockRestore() + }) +}) + describe('Run-level log upload', () => { const junitBasePath = './src/tests/fixtures/junit-xml' const allureBasePath = './src/tests/fixtures/allure' diff --git a/src/utils/result-upload/MarkerParser.ts b/src/utils/result-upload/MarkerParser.ts index c3e5702..292219e 100644 --- a/src/utils/result-upload/MarkerParser.ts +++ b/src/utils/result-upload/MarkerParser.ts @@ -150,9 +150,10 @@ export class MarkerParser { * Used by ResultUploader to map results → run test cases. */ nameMatchesTCase(name: string, projectCode: string, seq: number): boolean { - // 1. Hyphenated: case-insensitive check with hyphenated marker (e.g., TEST-002) + // 1. Hyphenated: case-insensitive check with exact hyphenated marker using word boundaries const hyphenated = formatMarker(projectCode, seq) - if (name.toLowerCase().includes(hyphenated.toLowerCase())) { + const hyphenatedPattern = new RegExp(`\\b${hyphenated}\\b`, 'i') + if (hyphenatedPattern.test(name)) { return true } diff --git a/src/utils/result-upload/ResultUploader.ts b/src/utils/result-upload/ResultUploader.ts index 6c1c2a4..61aa3ac 100644 --- a/src/utils/result-upload/ResultUploader.ts +++ b/src/utils/result-upload/ResultUploader.ts @@ -52,7 +52,8 @@ export class ResultUploader { if (mappedResults.length) { await this.uploadTestCases(mappedResults) - console.log(`Uploaded ${mappedResults.length} test cases`) + const uniqueTCases = new Set(mappedResults.map((r) => r.tcase.id)).size + console.log(`Uploaded ${mappedResults.length} results to ${uniqueTCases} test cases`) } } @@ -208,30 +209,39 @@ ${chalk.yellow('To fix this issue, choose one of the following options:')} return results } - // Collect all attachments from all test cases - const allAttachments: Array<{ - attachment: Attachment - tcaseIndex: number - }> = [] + // Collect unique attachments, deduplicating by file path + const uniqueAttachments = new Map() results.forEach((item, index) => { item.result.attachments.forEach((attachment) => { if (attachment.buffer !== null) { - allAttachments.push({ attachment, tcaseIndex: index }) + const existing = uniqueAttachments.get(attachment.filePath) + if (existing) { + existing.tcaseIndices.push(index) + } else { + uniqueAttachments.set(attachment.filePath, { + attachment, + tcaseIndices: [index], + }) + } } }) }) - if (allAttachments.length === 0) { + if (uniqueAttachments.size === 0) { return results } + const uniqueEntries = [...uniqueAttachments.values()] + const totalRefs = uniqueEntries.reduce((sum, e) => sum + e.tcaseIndices.length, 0) + const duplicateCount = totalRefs - uniqueEntries.length + // Group attachments into batches where total size <= MAX_BATCH_SIZE_BYTES - const batches: Array = [] - let currentBatch: typeof allAttachments = [] + const batches: Array = [] + let currentBatch: typeof uniqueEntries = [] let currentBatchSize = 0 - for (const item of allAttachments) { + for (const item of uniqueEntries) { const size = item.attachment.buffer!.byteLength if ( currentBatch.length > 0 && @@ -251,7 +261,8 @@ ${chalk.yellow('To fix this issue, choose one of the following options:')} // Upload batches concurrently with progress tracking let uploadedCount = 0 - loader.start(`Uploading attachments: 0/${allAttachments.length} files uploaded`) + const duplicateMsg = duplicateCount > 0 ? ` (${duplicateCount} duplicates skipped)` : '' + loader.start(`Uploading attachments: 0/${uniqueEntries.length} files uploaded${duplicateMsg}`) const batchResults = await this.processConcurrently( batches, @@ -265,11 +276,11 @@ ${chalk.yellow('To fix this issue, choose one of the following options:')} uploadedCount += batch.length loader.setText( - `Uploading attachments: ${uploadedCount}/${allAttachments.length} files uploaded` + `Uploading attachments: ${uploadedCount}/${uniqueEntries.length} files uploaded${duplicateMsg}` ) return batch.map((item, i) => ({ - tcaseIndex: item.tcaseIndex, + tcaseIndices: item.tcaseIndices, url: uploaded[i].url, name: item.attachment.filename, })) @@ -278,14 +289,16 @@ ${chalk.yellow('To fix this issue, choose one of the following options:')} ) loader.stop() - // Flatten batch results and group by test case index + // Flatten batch results and distribute URLs to all referencing test cases const attachmentsByTCase = new Map>() for (const batchResult of batchResults) { - for (const { tcaseIndex, url, name } of batchResult) { - if (!attachmentsByTCase.has(tcaseIndex)) { - attachmentsByTCase.set(tcaseIndex, []) + for (const { tcaseIndices, url, name } of batchResult) { + for (const tcaseIndex of tcaseIndices) { + if (!attachmentsByTCase.has(tcaseIndex)) { + attachmentsByTCase.set(tcaseIndex, []) + } + attachmentsByTCase.get(tcaseIndex)!.push({ url, name }) } - attachmentsByTCase.get(tcaseIndex)!.push({ url, name }) } } diff --git a/src/utils/result-upload/parsers/playwrightJsonParser.ts b/src/utils/result-upload/parsers/playwrightJsonParser.ts index e05f651..a181cc8 100644 --- a/src/utils/result-upload/parsers/playwrightJsonParser.ts +++ b/src/utils/result-upload/parsers/playwrightJsonParser.ts @@ -104,35 +104,40 @@ export const parsePlaywrightJson: Parser = async ( return // Can this happen? } - const markerFromAnnotations = getTCaseMarkerFromAnnotations(test.annotations) // What about result.annotations? + const markers = getAllTCaseMarkersFromAnnotations(test.annotations) // What about result.annotations? const status = mapPlaywrightStatus(test.status) - const numTestcases = testcases.push({ - // Use markerFromAnnotations as name prefix, so that it takes precedence over any - // other marker present. Prefixing it to name also helps in detectProjectCode - name: markerFromAnnotations - ? `${markerFromAnnotations}: ${titlePrefix}${spec.title}` - : `${titlePrefix}${spec.title}`, - folder: topLevelSuite, - status, - message: buildMessage(result, status, options), - timeTaken: result.duration, - attachments: [], - }) - - const attachmentPaths = [] + const message = buildMessage(result, status, options) + const baseName = `${titlePrefix}${spec.title}` + + const attachmentPaths: string[] = [] for (const out of result.attachments || []) { if (out.path) { attachmentPaths.push(out.path) } } - attachmentsPromises.push({ - index: numTestcases - 1, - // Attachment paths are absolute, but in tests we are using relative paths - promise: getAttachments( - attachmentPaths, - attachmentPaths[0]?.startsWith('/') ? undefined : attachmentBaseDirectory - ), - }) + const attachmentPromise = getAttachments( + attachmentPaths, + attachmentPaths[0]?.startsWith('/') ? undefined : attachmentBaseDirectory + ) + + // Fan out: one TestCaseResult per annotation, or one with no prefix if no annotations + const resultNames = + markers.length > 0 ? markers.map((marker) => `${marker}: ${baseName}`) : [baseName] + + for (const name of resultNames) { + const numTestcases = testcases.push({ + name, + folder: topLevelSuite, + status, + message, + timeTaken: result.duration, + attachments: [], + }) + attachmentsPromises.push({ + index: numTestcases - 1, + promise: attachmentPromise, + }) + } } // Recursively process nested suites @@ -164,15 +169,17 @@ export const parsePlaywrightJson: Parser = async ( return { testCaseResults: testcases, runFailureLogs: runFailureLogParts.join('') } } -const getTCaseMarkerFromAnnotations = (annotations: Annotation[]) => { +const getAllTCaseMarkersFromAnnotations = (annotations: Annotation[]): string[] => { + const markers: string[] = [] for (const annotation of annotations) { if (annotation.type.toLowerCase().includes('test case') && annotation.description) { const res = parseTCaseUrl(annotation.description) if (res) { - return formatMarker(res.project, res.tcaseSeq) + markers.push(formatMarker(res.project, res.tcaseSeq)) } } } + return markers } const mapPlaywrightStatus = (status: Status): ResultStatus => { diff --git a/src/utils/result-upload/types.ts b/src/utils/result-upload/types.ts index a16f5da..b2cc2ce 100644 --- a/src/utils/result-upload/types.ts +++ b/src/utils/result-upload/types.ts @@ -2,6 +2,7 @@ import { ResultStatus } from '../../api/schemas' export interface Attachment { filename: string + filePath: string buffer: Buffer | null error: Error | null } diff --git a/src/utils/result-upload/utils.ts b/src/utils/result-upload/utils.ts index db4f748..7e68a38 100644 --- a/src/utils/result-upload/utils.ts +++ b/src/utils/result-upload/utils.ts @@ -1,5 +1,5 @@ import { readFile } from 'fs/promises' -import path, { basename } from 'path' +import path, { basename, join } from 'path' import { Attachment } from './types' const getFile = async (filePath: string, basePath?: string): Promise => { @@ -26,6 +26,7 @@ export const getAttachments = async ( return Promise.allSettled(filePaths.map((p) => getFile(p, basePath))).then((results) => { return results.map((p, i) => ({ filename: basename(filePaths[i]), + filePath: basePath ? join(basePath, filePaths[i]) : filePaths[i], buffer: p.status === 'fulfilled' ? p.value : null, error: p.status === 'fulfilled' ? null : p.reason, })) From 0d9c29e382f8862894bffabb9ef221d9ed30c313 Mon Sep 17 00:00:00 2001 From: Ramil Amparo Date: Wed, 1 Apr 2026 14:21:36 +0400 Subject: [PATCH 2/6] Fix upload count --- src/tests/result-upload.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tests/result-upload.spec.ts b/src/tests/result-upload.spec.ts index 31570d7..d858eb4 100644 --- a/src/tests/result-upload.spec.ts +++ b/src/tests/result-upload.spec.ts @@ -158,10 +158,10 @@ const countRunLogApiCalls = () => const countIndividualFileUploads = () => { let count = 0 - server.events.on('response:mocked', async (e) => { + server.events.on('request:start', async (e) => { if (!new URL(e.request.url).pathname.endsWith('/file/batch')) return - const body = (await e.response.clone().json()) as { files: unknown[] } - count += body.files.length + const form = await e.request.clone().formData() + count += form.getAll('files').length }) return () => count } From ee3888e3c609fbc4fa3a222cfac58e1d887677ff Mon Sep 17 00:00:00 2001 From: Ramil Amparo Date: Wed, 1 Apr 2026 14:30:10 +0400 Subject: [PATCH 3/6] Dedup annotations --- src/tests/playwright-json-parsing.spec.ts | 15 +++++++-------- src/tests/result-upload.spec.ts | 12 ++++++------ .../result-upload/parsers/playwrightJsonParser.ts | 7 +++++-- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/tests/playwright-json-parsing.spec.ts b/src/tests/playwright-json-parsing.spec.ts index 630f0f0..5e9ec7d 100644 --- a/src/tests/playwright-json-parsing.spec.ts +++ b/src/tests/playwright-json-parsing.spec.ts @@ -388,21 +388,20 @@ describe('Playwright JSON parsing', () => { skipStderr: 'never', }) - // Fixture has 1 test with 3 annotations (2 for 10427, 1 for 10428) + 1 test with no annotations = 4 results - expect(testcases).toHaveLength(4) + // Fixture has 1 test with 3 annotations (2x 10427 deduped to 1, plus 10428) + 1 test with no annotations = 3 results + expect(testcases).toHaveLength(3) expect(testcases[0].name).toBe('TEST-10427: Login flow covers multiple cases') - expect(testcases[1].name).toBe('TEST-10427: Login flow covers multiple cases') - expect(testcases[2].name).toBe('TEST-10428: Login flow covers multiple cases') - expect(testcases[3].name).toBe('Navigation bar items TEST-006') + expect(testcases[1].name).toBe('TEST-10428: Login flow covers multiple cases') + expect(testcases[2].name).toBe('Navigation bar items TEST-006') - // The three fan-out entries share the same status, duration, folder - for (const tc of testcases.slice(0, 3)) { + // The two fan-out entries share the same status, duration, folder + for (const tc of testcases.slice(0, 2)) { expect(tc.status).toBe('passed') expect(tc.timeTaken).toBe(2500) expect(tc.folder).toBe('multi-annotation.spec.ts') } - // All four entries have attachments from the fixture + // All three entries have attachments from the fixture for (const tc of testcases) { expect(tc.attachments).toHaveLength(1) } diff --git a/src/tests/result-upload.spec.ts b/src/tests/result-upload.spec.ts index d858eb4..66e64c3 100644 --- a/src/tests/result-upload.spec.ts +++ b/src/tests/result-upload.spec.ts @@ -652,9 +652,9 @@ describe('Multi-annotation Playwright upload', () => { `playwright-json-upload -r ${runURL} --attachments ./src/tests/fixtures/playwright-json/multi-annotation-with-attachments.json` ) - // The fixture has 1 test with 3 annotations (fan-out to 3 results) and 1 test with no - // annotations. All 4 results reference the same attachment file. - // Only 1 unique file should be uploaded, not 4. + // The fixture has 1 test with 3 annotations (2x 10427 deduped, 1x 10428 = 2 fan-out results) + // and 1 test with no annotations. All 3 results reference the same attachment file. + // Only 1 unique file should be uploaded, not 3. expect(totalFilesUploaded()).toBe(1) }) @@ -665,10 +665,10 @@ describe('Multi-annotation Playwright upload', () => { `playwright-json-upload -r ${runURL} ./src/tests/fixtures/playwright-json/multi-annotation-with-attachments.json` ) - // Fixture: 3 fan-out results (2x TEST-10427, 1x TEST-10428) + 1 regular (TEST-006) - // = 4 results mapping to 3 unique test cases + // Fixture: 2 fan-out results (10427 deduped, 10428) + 1 regular (TEST-006) + // = 3 results mapping to 3 unique test cases expect(logSpy).toHaveBeenCalledWith( - expect.stringContaining('Uploaded 4 results to 3 test cases') + expect.stringContaining('Uploaded 3 results to 3 test cases') ) logSpy.mockRestore() }) diff --git a/src/utils/result-upload/parsers/playwrightJsonParser.ts b/src/utils/result-upload/parsers/playwrightJsonParser.ts index a181cc8..17a647e 100644 --- a/src/utils/result-upload/parsers/playwrightJsonParser.ts +++ b/src/utils/result-upload/parsers/playwrightJsonParser.ts @@ -120,9 +120,12 @@ export const parsePlaywrightJson: Parser = async ( attachmentPaths[0]?.startsWith('/') ? undefined : attachmentBaseDirectory ) - // Fan out: one TestCaseResult per annotation, or one with no prefix if no annotations + // Fan out: one TestCaseResult per unique annotation, or one with no prefix if no annotations + const uniqueMarkers = [...new Set(markers)] const resultNames = - markers.length > 0 ? markers.map((marker) => `${marker}: ${baseName}`) : [baseName] + uniqueMarkers.length > 0 + ? uniqueMarkers.map((marker) => `${marker}: ${baseName}`) + : [baseName] for (const name of resultNames) { const numTestcases = testcases.push({ From 1969a9ed4e2d598bf228443ccc0037fc15859cde Mon Sep 17 00:00:00 2001 From: Ramil Amparo Date: Wed, 1 Apr 2026 14:38:02 +0400 Subject: [PATCH 4/6] Fixes --- src/utils/result-upload/parsers/playwrightJsonParser.ts | 3 ++- src/utils/result-upload/utils.ts | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/utils/result-upload/parsers/playwrightJsonParser.ts b/src/utils/result-upload/parsers/playwrightJsonParser.ts index 17a647e..858fbac 100644 --- a/src/utils/result-upload/parsers/playwrightJsonParser.ts +++ b/src/utils/result-upload/parsers/playwrightJsonParser.ts @@ -159,7 +159,8 @@ export const parsePlaywrightJson: Parser = async ( const attachments = await Promise.all(attachmentsPromises.map((p) => p.promise)) attachments.forEach((tcaseAttachment, i) => { const tcaseIndex = attachmentsPromises[i].index - testcases[tcaseIndex].attachments = tcaseAttachment + // Clone to avoid affecting other test cases if any downstream code mutates the array + testcases[tcaseIndex].attachments = [...tcaseAttachment] }) // Build runFailureLogs from top-level errors diff --git a/src/utils/result-upload/utils.ts b/src/utils/result-upload/utils.ts index 7e68a38..25a5d5f 100644 --- a/src/utils/result-upload/utils.ts +++ b/src/utils/result-upload/utils.ts @@ -1,5 +1,5 @@ import { readFile } from 'fs/promises' -import path, { basename, join } from 'path' +import path, { basename, resolve } from 'path' import { Attachment } from './types' const getFile = async (filePath: string, basePath?: string): Promise => { @@ -26,7 +26,7 @@ export const getAttachments = async ( return Promise.allSettled(filePaths.map((p) => getFile(p, basePath))).then((results) => { return results.map((p, i) => ({ filename: basename(filePaths[i]), - filePath: basePath ? join(basePath, filePaths[i]) : filePaths[i], + filePath: resolve(basePath ?? '.', filePaths[i]), buffer: p.status === 'fulfilled' ? p.value : null, error: p.status === 'fulfilled' ? null : p.reason, })) From 810984d96f48f755c461d1b010ba57c3716a98cd Mon Sep 17 00:00:00 2001 From: Ramil Amparo Date: Wed, 1 Apr 2026 14:54:31 +0400 Subject: [PATCH 5/6] Fix test case matching --- src/tests/marker-parser.spec.ts | 8 ++++++++ src/utils/result-upload/MarkerParser.ts | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/tests/marker-parser.spec.ts b/src/tests/marker-parser.spec.ts index c440dce..e444ca5 100644 --- a/src/tests/marker-parser.spec.ts +++ b/src/tests/marker-parser.spec.ts @@ -249,6 +249,14 @@ describe('nameMatchesTCase', () => { test('marker surrounded by parens does not prefix-match', () => { expect(playwright.nameMatchesTCase('some test (QS1-10427)', 'QS1', 104)).toBe(false) }) + + test('marker adjacent to underscore still matches', () => { + expect(playwright.nameMatchesTCase('test_QS1-104_login', 'QS1', 104)).toBe(true) + }) + + test('marker adjacent to bracket still matches', () => { + expect(playwright.nameMatchesTCase('case[QS1-104]', 'QS1', 104)).toBe(true) + }) }) }) diff --git a/src/utils/result-upload/MarkerParser.ts b/src/utils/result-upload/MarkerParser.ts index 292219e..acf4a7e 100644 --- a/src/utils/result-upload/MarkerParser.ts +++ b/src/utils/result-upload/MarkerParser.ts @@ -152,7 +152,7 @@ export class MarkerParser { nameMatchesTCase(name: string, projectCode: string, seq: number): boolean { // 1. Hyphenated: case-insensitive check with exact hyphenated marker using word boundaries const hyphenated = formatMarker(projectCode, seq) - const hyphenatedPattern = new RegExp(`\\b${hyphenated}\\b`, 'i') + const hyphenatedPattern = new RegExp(`(? Date: Wed, 1 Apr 2026 15:08:08 +0400 Subject: [PATCH 6/6] Escape html for attachments --- src/utils/result-upload/ResultUploader.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/utils/result-upload/ResultUploader.ts b/src/utils/result-upload/ResultUploader.ts index 61aa3ac..4a17465 100644 --- a/src/utils/result-upload/ResultUploader.ts +++ b/src/utils/result-upload/ResultUploader.ts @@ -1,5 +1,6 @@ import { Arguments } from 'yargs' import chalk from 'chalk' +import escapeHtml from 'escape-html' import { RunTCase } from '../../api/schemas' import { parseRunUrl, printError, printErrorThenExit, twirlLoader } from '../misc' import { Api, createApi } from '../../api' @@ -404,6 +405,6 @@ interface TCaseWithResult { const makeListHtml = (list: { name: string; url: string }[]) => { return `` }