From 1a9ec2f662f7b12bd2bdbdf667eb70d7c0bbab69 Mon Sep 17 00:00:00 2001 From: Satvik Choudhary Date: Mon, 6 Apr 2026 12:10:42 +0530 Subject: [PATCH 1/5] Fix marker resolution and duplicate mapping --- QAS_CLI_ISSUES_BRIEF.md | 117 ++++++++++ QAS_CLI_ISSUES_REPORT.md | 221 ++++++++++++++++++ README.md | 4 +- src/commands/resultUpload.ts | 3 +- .../describe-marker-collision.json | 39 ++++ .../describe-marker-unmatched.json | 39 ++++ src/tests/playwright-json-parsing.spec.ts | 120 ++++++++++ src/tests/result-upload.spec.ts | 94 ++++++++ src/utils/result-upload/MarkerParser.ts | 15 +- .../ResultUploadCommandHandler.ts | 124 +++++++++- src/utils/result-upload/ResultUploader.ts | 56 ++--- src/utils/result-upload/mapping.ts | 63 +++++ .../result-upload/parsers/allureParser.ts | 23 +- .../result-upload/parsers/junitXmlParser.ts | 4 + .../parsers/playwrightJsonParser.ts | 49 +++- src/utils/result-upload/types.ts | 15 +- 16 files changed, 923 insertions(+), 63 deletions(-) create mode 100644 QAS_CLI_ISSUES_BRIEF.md create mode 100644 QAS_CLI_ISSUES_REPORT.md create mode 100644 src/tests/fixtures/playwright-json/describe-marker-collision.json create mode 100644 src/tests/fixtures/playwright-json/describe-marker-unmatched.json create mode 100644 src/utils/result-upload/mapping.ts diff --git a/QAS_CLI_ISSUES_BRIEF.md b/QAS_CLI_ISSUES_BRIEF.md new file mode 100644 index 0000000..cd4e29a --- /dev/null +++ b/QAS_CLI_ISSUES_BRIEF.md @@ -0,0 +1,117 @@ +# qas-cli: incorrect Playwright upload mapping to QA Sphere runs + +## Summary + +`qas-cli playwright-json-upload` can report a successful upload while mapping many Playwright results to the wrong QA Sphere test cases. + +In the reproduced upload: + +- Playwright run: `142` tests total, `140 passed`, `2 failed` +- CLI upload output: `Uploaded 139 test cases` +- QA Sphere run summary after upload: only `33 passed`, `1 failed` + +Root cause: + +- the uploader matches hyphenated markers with substring logic, so `QS1-10427` incorrectly matches `QS1-104` +- the Playwright parser only uses the first `test case` annotation and drops additional ones + +## Repro + +Using the Chromium Playwright JSON report from `tms-2` and uploading to existing run `QS1/run/53`: + +- `139` results were considered matched +- but they collapsed onto only `34` unique run test cases + +Observed collisions: + +- `QS1-104` absorbed `58` results +- `QS1-103` absorbed `25` +- `QS1-106` absorbed `19` +- `QS1-105` absorbed `4` + +This produced the exact run summary seen in QA Sphere: `33 passed / 1 failed`. + +## Issues + +### 1. Exact marker matching is broken + +File: + +- [src/utils/result-upload/MarkerParser.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/MarkerParser.ts#L152) + +Current code: + +- `name.toLowerCase().includes(hyphenated.toLowerCase())` + +Problem: + +- `QS1-10427` matches `QS1-104` +- `QS1-10775` matches `QS1-107` + +Expected: + +- hyphenated markers must match exactly with boundaries +- `QS1-104` must not match `QS1-10427` + +### 2. Playwright multi-annotation support is incomplete + +File: + +- [src/utils/result-upload/playwrightJsonParser.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/playwrightJsonParser.ts#L106) + +Problem: + +- only the first QA Sphere `test case` annotation is used +- additional annotations on the same test are dropped + +Evidence from the reproduced report: + +- `138` tests had annotations +- `26` tests had multiple annotations +- examples included tests with `2`, `3`, `4`, `6`, `7`, `8`, and `10` QA Sphere references + +Expected: + +- one Playwright test with multiple QA Sphere annotations should fan out into multiple upload results, one per annotation + +### 3. Upload success messaging is misleading + +File: + +- [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L31) + +Problem: + +- final message says `Uploaded 139 test cases` +- in reality those were `139` mapped result records targeting only `34` unique run test cases +- duplicate mapping collisions are not surfaced + +Expected: + +- output should distinguish: + - number of source results + - number of mapped results + - number of unique target test cases +- uploader should warn or fail when many results collapse onto the same target case unexpectedly + +## Acceptance Criteria + +1. `nameMatchesTCase()` uses exact matching for hyphenated markers and no longer allows prefix collisions. +2. Playwright parsing supports multiple `test case` annotations by generating one upload result per referenced QA Sphere case. +3. Uploader output clearly reports mapped results vs unique target test cases. +4. Regression tests cover: + - `QS1-104` vs `QS1-10427` + - single vs multiple Playwright annotations + - duplicate-mapping detection or reporting + +## Notes + +Three unmatched tests in the reproduced run were genuinely unlinked and are not part of this bug: + +- `should show error for invalid credentials` +- `Complete smoke test of basic functionality` +- `should show no run cards for project with no assigned runs` + +Full report: + +- [QAS_CLI_ISSUES_REPORT.md](/Users/apple/Github/Hypersequent/qas-cli/QAS_CLI_ISSUES_REPORT.md) diff --git a/QAS_CLI_ISSUES_REPORT.md b/QAS_CLI_ISSUES_REPORT.md new file mode 100644 index 0000000..ef6dabc --- /dev/null +++ b/QAS_CLI_ISSUES_REPORT.md @@ -0,0 +1,221 @@ +# qas-cli Issue Report + +Date: 2026-03-20 + +## Scope + +This report covers issues found while uploading the Chromium Playwright e2e run from `tms-2` into an existing QA Sphere run with `qas-cli playwright-json-upload`. + +Observed local run summary: + +- `142` Playwright tests total +- `140 passed` +- `2 failed` +- `3` tests had no QA Sphere marker/annotation and were intentionally unmatched + +Observed uploader output: + +- first upload failed on the 3 unmatched tests +- retry with `--ignore-unmatched` reported `Uploaded 139 test cases` + +Observed QA Sphere run summary after upload: + +- `33 passed` +- `1 failed` +- `762 open` + +That mismatch is explained by the issues below. + +## Findings + +### 1. Critical: `nameMatchesTCase()` uses substring matching for hyphenated markers + +Files: + +- [src/utils/result-upload/MarkerParser.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/MarkerParser.ts#L152) +- [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L322) + +Current behavior: + +- `nameMatchesTCase()` builds a canonical marker like `QS1-104` +- it then checks `name.toLowerCase().includes(hyphenated.toLowerCase())` + +That is incorrect for hyphenated markers because: + +- `QS1-10427` incorrectly matches `QS1-104` +- `QS1-10775` incorrectly matches `QS1-107` +- any shorter prefix test case already present in the run can absorb unrelated longer markers + +### Evidence + +Replaying the current uploader logic against the generated Playwright JSON and the existing run's test cases produced: + +- `142` total Playwright results +- `139` considered matched +- only `34` unique run test cases targeted + +The matched `seq` set was: + +- `8, 13, 15, 17, 19, 21, 32, 33, 34, 35, 36, 37, 38, 39, 103, 104, 105, 106, 107, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 223, 365` + +The worst collisions were: + +- `QS1-104` matched `58` uploaded results +- `QS1-103` matched `25` +- `QS1-106` matched `19` +- `QS1-105` matched `4` + +This exactly explains why the run summary only showed `33 passed / 1 failed`: the results were collapsed onto a small set of prefix case IDs already present in the run. + +### Impact + +- Results are uploaded to the wrong test cases +- Existing run summaries become misleading +- Failures can be assigned to unrelated test cases +- The CLI can appear to succeed while corrupting run-level reporting + +### Suggested fix + +Replace substring matching with exact marker matching using boundaries. + +Expected semantics: + +- `QS1-104` should match only `QS1-104` +- it must not match `QS1-10427` + +Reasonable implementation options: + +- use a regex with boundaries around the full marker +- or parse markers from the result name first, then compare normalized marker values exactly + +Suggested tests: + +- `nameMatchesTCase('QS1-10427: some test', 'QS1', 104)` should be `false` +- `nameMatchesTCase('QS1-104: some test', 'QS1', 104)` should be `true` +- `nameMatchesTCase('some test (QS1-10427)', 'QS1', 10427)` should be `true` +- `nameMatchesTCase('some test (QS1-10427)', 'QS1', 104)` should be `false` + +### 2. High: Playwright parser only uses the first `test case` annotation + +Files: + +- [src/utils/result-upload/playwrightJsonParser.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/playwrightJsonParser.ts#L106) +- [src/utils/result-upload/playwrightJsonParser.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/playwrightJsonParser.ts#L159) + +Current behavior: + +- `getTCaseMarkerFromAnnotations()` returns only a single marker +- `parsePlaywrightJson()` prefixes that one marker into the result name +- any additional QA Sphere annotations on the same Playwright test are discarded + +### Evidence + +In the generated Playwright JSON: + +- `138` tests had at least one annotation +- `26` tests had multiple QA Sphere annotations + +Examples: + +- one test had `7` annotations: `QS1-10427`, `QS1-10428`, `QS1-10429`, `QS1-10430`, `QS1-10668`, `QS1-10669`, `QS1-10670` +- another had `8` annotations: `QS1-10591` through `QS1-10598` +- another had `6` annotations: `QS1-10775` through `QS1-10780` + +Today only the first annotation is used, so the rest are lost before matching even starts. + +### Impact + +- Multi-case Playwright tests cannot upload all referenced QA Sphere cases +- Uploaded coverage is incomplete even if marker matching is fixed +- Complex scenario tests appear under only one case instead of all declared cases + +### Suggested fix + +Change the Playwright parser to return multiple `TestCaseResult` entries when a test has multiple QA Sphere annotations. + +Expected behavior: + +- one Playwright test with `N` QA Sphere annotations should fan out into `N` result entries +- each entry should carry the same status, attachments, comment, and timing +- each entry should map to its own QA Sphere test case + +Suggested tests: + +- single annotation still produces one result +- multiple annotations produce multiple results +- annotation order does not drop later markers +- annotations should take precedence over name markers when both exist + +### 3. Medium: uploader messaging is misleading and hides destructive fan-in + +Files: + +- [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L31) +- [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L272) +- [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L322) + +Current behavior: + +- uploader logs `Uploaded ${mappedResults.length} test cases` +- in this repro that message was `Uploaded 139 test cases` +- but those `139` mapped results collapsed onto only `34` unique run test cases because of issue 1 + +The message is misleading even aside from the bug: + +- it is counting uploaded result records, not distinct test cases +- it does not report duplicate mappings +- it does not warn when many source results fan into a small set of target test cases + +### Impact + +- false confidence after upload +- hard to spot mapping corruption +- user sees success in CLI but wrong numbers in QA Sphere UI + +### Suggested fix + +Improve diagnostics before and after upload: + +- log both `mapped results` and `unique target test cases` +- if multiple source results map to the same run test case, warn or fail unless explicitly allowed +- change final message to `Uploaded X results to Y test cases` + +Also consider validating API responses more strictly: + +- compare requested batch size with returned ID count +- surface partial-acceptance conditions clearly + +Suggested tests: + +- duplicate mappings should be visible in output +- final message should distinguish results vs distinct cases + +## Not Bugs + +These were investigated and should not be treated as defects: + +- Uploading to an existing run only maps against test cases already present in that run. + This is by design in [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L32). + +- The 3 unmatched Playwright tests in this run were genuinely unlinked: + - `should show error for invalid credentials` + - `Complete smoke test of basic functionality` + - `should show no run cards for project with no assigned runs` + +## Recommended Fix Order + +1. Fix exact marker matching in `nameMatchesTCase()` +2. Add regression tests for prefix collisions +3. Fix Playwright multi-annotation fan-out +4. Add regression tests for multi-annotation parsing and upload mapping +5. Improve uploader diagnostics and final reporting + +## Minimal Repro Summary + +Using the generated Playwright JSON from the Chromium run: + +- the current parser/uploader mapped `139` results +- but only `34` unique run test cases were targeted +- most high-numbered `QS1-*` markers were incorrectly absorbed by short prefix IDs like `QS1-103`, `QS1-104`, `QS1-105`, `QS1-106`, `QS1-107` + +That is the main reason the QA Sphere UI showed only `33 passed / 1 failed` instead of reflecting the broader Chromium run. diff --git a/README.md b/README.md index 2be0015..6fbfc85 100644 --- a/README.md +++ b/README.md @@ -182,7 +182,7 @@ There are two modes for uploading results using the commands: - `--run-name` - Optional name template for creating new test run. It supports `{env:VAR}`, `{YYYY}`, `{YY}`, `{MM}`, `{MMM}`, `{DD}`, `{HH}`, `{hh}`, `{mm}`, `{ss}`, `{AMPM}` placeholders (default: `Automated test run - {MMM} {DD}, {YYYY}, {hh}:{mm}:{ss} {AMPM}`) - `--create-tcases` - Automatically create test cases in QA Sphere for results that don't have valid test case markers. A mapping file (`qasphere-automapping-YYYYMMDD-HHmmss.txt`) is generated showing the sequence numbers assigned to each new test case (default: `false`) - `--attachments` - Try to detect and upload any attachments with the test result -- `--force` - Ignore API request errors, invalid test cases, or attachments +- `--force` - Ignore API request errors, invalid or duplicate test case mappings, or attachments - `--ignore-unmatched` - Suppress individual unmatched test messages, show summary only - `--skip-report-stdout` - Control when to skip stdout blocks from test report (choices: `on-success`, `never`; default: `never`) - `--skip-report-stderr` - Control when to skip stderr blocks from test report (choices: `on-success`, `never`; default: `never`) @@ -297,7 +297,7 @@ Ensure the required environment variables are defined before running these comma ## Test Report Requirements -The QAS CLI maps test results from your reports (JUnit XML, Playwright JSON, or Allure) to corresponding test cases in QA Sphere. If a test result lacks a valid marker/reference, the CLI will display an error unless you use `--create-tcases` to automatically create test cases, or `--ignore-unmatched`/`--force` to skip unmatched results. +The QAS CLI maps test results from your reports (JUnit XML, Playwright JSON, or Allure) to corresponding test cases in QA Sphere. If a test result lacks a valid marker/reference, or multiple results resolve to the same run test case, the CLI will display an error unless you use `--create-tcases` to automatically create test cases, or `--ignore-unmatched`/`--force` to bypass the mapping issue. ### JUnit XML diff --git a/src/commands/resultUpload.ts b/src/commands/resultUpload.ts index 8b8fd53..92b4eeb 100644 --- a/src/commands/resultUpload.ts +++ b/src/commands/resultUpload.ts @@ -84,7 +84,8 @@ export class ResultUploadCommandModule implements CommandModule { // 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[0].marker).toEqual({ projectCode: 'TEST', seq: 10427 }) + expect(testcases[0].markerResolution).toBe('resolved') expect(testcases[1].name).toBe('TEST-10428: Login flow covers multiple cases') + expect(testcases[1].marker).toEqual({ projectCode: 'TEST', seq: 10428 }) + expect(testcases[1].markerResolution).toBe('resolved') expect(testcases[2].name).toBe('Navigation bar items TEST-006') + expect(testcases[2].marker).toEqual({ projectCode: 'TEST', seq: 6 }) + expect(testcases[2].markerResolution).toBe('resolved') // The two fan-out entries share the same status, duration, folder for (const tc of testcases.slice(0, 2)) { @@ -454,6 +460,8 @@ describe('Playwright JSON parsing', () => { expect(testcases).toHaveLength(1) expect(testcases[0].name).toBe('PRJ-100: Simple test') + expect(testcases[0].marker).toEqual({ projectCode: 'PRJ', seq: 100 }) + expect(testcases[0].markerResolution).toBe('resolved') }) test('Should fan out by annotations even when name has a marker', async () => { @@ -509,6 +517,118 @@ describe('Playwright JSON parsing', () => { 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') + expect(testcases[0].marker).toEqual({ projectCode: 'PRJ', seq: 100 }) + expect(testcases[1].marker).toEqual({ projectCode: 'PRJ', seq: 200 }) + expect(testcases[0].markerResolution).toBe('resolved') + expect(testcases[1].markerResolution).toBe('resolved') + }) + + test('Should extract fallback marker from spec title, not describe titles', async () => { + const jsonContent = JSON.stringify({ + suites: [ + { + title: 'describe-collision.spec.ts', + specs: [], + suites: [ + { + title: 'Test Case Folders (TEST-002, TEST-003)', + specs: [ + { + title: 'should create a new root folder (TEST-004)', + tags: [], + tests: [ + { + annotations: [], + 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( + 'Test Case Folders (TEST-002, TEST-003) › should create a new root folder (TEST-004)' + ) + expect(testcases[0].marker).toEqual({ projectCode: 'TEST', seq: 4 }) + expect(testcases[0].markerResolution).toBe('resolved') + }) + + test('Should keep marker unresolved-none when only describe title contains markers', async () => { + const jsonContent = JSON.stringify({ + suites: [ + { + title: 'describe-only-markers.spec.ts', + specs: [], + suites: [ + { + title: 'Dashboard Projects (TEST-034, TEST-035, TEST-365)', + specs: [ + { + title: 'viewer should not see project management actions', + tags: [], + tests: [ + { + annotations: [], + 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( + 'Dashboard Projects (TEST-034, TEST-035, TEST-365) › viewer should not see project management actions' + ) + expect(testcases[0].marker).toBeNull() + expect(testcases[0].markerResolution).toBe('resolved-none') }) test('Should map test status correctly', async () => { diff --git a/src/tests/result-upload.spec.ts b/src/tests/result-upload.spec.ts index e04a144..a7ccb5c 100644 --- a/src/tests/result-upload.spec.ts +++ b/src/tests/result-upload.spec.ts @@ -6,6 +6,7 @@ import { run } from '../commands/main' import { PaginatedResponse } from '../api/schemas' import { CreateTCasesRequest, CreateTCasesResponse, TCase } from '../api/tcases' import { Folder } from '../api/folders' +import { CreateResultsRequest } from '../api/results' import { DEFAULT_FOLDER_TITLE } from '../utils/result-upload/ResultUploadCommandHandler' import { setMaxResultsInRequest } from '../utils/result-upload/ResultUploader' import { runTestCases } from './fixtures/testcases' @@ -149,9 +150,22 @@ const countResultUploadApiCalls = () => countMockedApiCalls(server, (req) => new URL(req.url).pathname.endsWith('/result/batch')) const countCreateTCasesApiCalls = () => countMockedApiCalls(server, (req) => new URL(req.url).pathname.endsWith('/tcase/bulk')) +const countCreateRunApiCalls = () => + countMockedApiCalls(server, (req) => + new URL(req.url).pathname.endsWith(`/project/${projectCode}/run`) + ) const countRunLogApiCalls = () => countMockedApiCalls(server, (req) => new URL(req.url).pathname.endsWith(`/run/${runId}/log`)) +const captureResultBatchRequests = () => { + const requests: CreateResultsRequest[] = [] + server.events.on('request:start', async (e) => { + if (!new URL(e.request.url).pathname.endsWith('/result/batch')) return + requests.push((await e.request.clone().json()) as CreateResultsRequest) + }) + return () => requests +} + const countIndividualFileUploads = () => { let count = 0 server.events.on('request:start', async (e) => { @@ -670,6 +684,86 @@ describe('Multi-annotation Playwright upload', () => { }) }) +describe('Marker-based result mapping', () => { + test('Should map Playwright results using the spec marker, not markers from describe titles', async () => { + const getRequests = captureResultBatchRequests() + + await run( + `playwright-json-upload -r ${runURL} ./src/tests/fixtures/playwright-json/describe-marker-collision.json` + ) + + const uploadedTcaseIds = getRequests() + .flatMap((request) => request.items) + .map((item) => item.tcaseId) + expect(uploadedTcaseIds).toEqual(['1CBd7QsjQ_qDrjofzXkMLqE']) + }) + + test('Should leave Playwright results unmatched when only describe titles contain markers', async () => { + const numResultUploadCalls = countResultUploadApiCalls() + + await expect( + run( + `playwright-json-upload -r ${runURL} ./src/tests/fixtures/playwright-json/describe-marker-unmatched.json` + ) + ).rejects.toThrowError() + + expect(numResultUploadCalls()).toBe(0) + }) + + test('Should fail when multiple results map to the same run test case', async () => { + const numResultUploadCalls = countResultUploadApiCalls() + await expect( + run( + `junit-upload -r ${runURL} ./src/tests/fixtures/junit-xml/matching-tcases.xml ./src/tests/fixtures/junit-xml/matching-tcases.xml` + ) + ).rejects.toThrowError() + expect(numResultUploadCalls()).toBe(0) + }) + + test('Should fail duplicate target mappings before creating a new run', async () => { + const numCreateRunCalls = countCreateRunApiCalls() + const numResultUploadCalls = countResultUploadApiCalls() + + await expect( + run( + `junit-upload --project-code ${projectCode} ./src/tests/fixtures/junit-xml/matching-tcases.xml ./src/tests/fixtures/junit-xml/matching-tcases.xml` + ) + ).rejects.toThrowError() + + expect(numCreateRunCalls()).toBe(0) + expect(numResultUploadCalls()).toBe(0) + }) + + test('Should allow duplicate target mappings with --force', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) + const numResultUploadCalls = countResultUploadApiCalls() + setMaxResultsInRequest(10) + + await run( + `junit-upload -r ${runURL} --force ./src/tests/fixtures/junit-xml/matching-tcases.xml ./src/tests/fixtures/junit-xml/matching-tcases.xml` + ) + + expect(numResultUploadCalls()).toBe(1) + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('Uploaded 10 results to 5 test cases') + ) + logSpy.mockRestore() + }) + + test('Should create a new run for forced duplicate mappings without failing after creation', async () => { + const numCreateRunCalls = countCreateRunApiCalls() + const numResultUploadCalls = countResultUploadApiCalls() + setMaxResultsInRequest(10) + + await run( + `junit-upload --project-code ${projectCode} --force ./src/tests/fixtures/junit-xml/matching-tcases.xml ./src/tests/fixtures/junit-xml/matching-tcases.xml` + ) + + expect(numCreateRunCalls()).toBe(1) + expect(numResultUploadCalls()).toBe(1) + }) +}) + 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 acf4a7e..cd298cb 100644 --- a/src/utils/result-upload/MarkerParser.ts +++ b/src/utils/result-upload/MarkerParser.ts @@ -1,3 +1,4 @@ +import type { TestCaseMarker } from './types' import { UploadCommandType } from './ResultUploadCommandHandler' const MARKER_SEP = `_` @@ -33,8 +34,7 @@ const execRegexWithPriority = ( export const formatMarker = (projectCode: string, seq: number) => `${projectCode}-${seq.toString().padStart(3, '0')}` -/** Extract and normalize a hyphenated marker like "TEST-002" from free text. */ -export const getMarkerFromText = (text: string | undefined): string | undefined => { +export const getParsedMarkerFromText = (text: string | undefined): TestCaseMarker | undefined => { if (!text) { return undefined } @@ -44,7 +44,16 @@ export const getMarkerFromText = (text: string | undefined): string | undefined return undefined } - return formatMarker(match[1], Number(match[2])) + return { + projectCode: match[1], + seq: Number(match[2]), + } +} + +/** Extract and normalize a hyphenated marker like "TEST-002" from free text. */ +export const getMarkerFromText = (text: string | undefined): string | undefined => { + const marker = getParsedMarkerFromText(text) + return marker ? formatMarker(marker.projectCode, marker.seq) : undefined } export class MarkerParser { diff --git a/src/utils/result-upload/ResultUploadCommandHandler.ts b/src/utils/result-upload/ResultUploadCommandHandler.ts index 1d9c402..d0df01a 100644 --- a/src/utils/result-upload/ResultUploadCommandHandler.ts +++ b/src/utils/result-upload/ResultUploadCommandHandler.ts @@ -6,7 +6,8 @@ import { parseRunUrl, printErrorThenExit, processTemplate } from '../misc' import { MarkerParser } from './MarkerParser' import { Api, createApi } from '../../api' import { TCase } from '../../api/tcases' -import { ParseResult, TestCaseResult } from './types' +import { ParseResult, TestCaseMarker, TestCaseResult } from './types' +import { DuplicateTCaseMapping, TCaseTarget, mapResolvedResultsToTCases } from './mapping' import { ResultUploader } from './ResultUploader' import { parseJUnitXml } from './parsers/junitXmlParser' import { parsePlaywrightJson } from './parsers/playwrightJsonParser' @@ -77,6 +78,7 @@ export class ResultUploadCommandHandler { private api: Api private baseUrl: string private markerParser: MarkerParser + private skipUploaderDuplicateValidation = false constructor( private type: UploadCommandType, @@ -111,6 +113,7 @@ export class ResultUploadCommandHandler { runId = urlParsed.run projectCode = urlParsed.project + this.resolveMarkers(fileResults, projectCode) } else { if (this.args.projectCode) { projectCode = this.args.projectCode as string @@ -127,7 +130,12 @@ export class ResultUploadCommandHandler { const resp = await this.getTCaseIds(projectCode, fileResults) fileResults = resp.fileResults - runId = await this.createNewRun(projectCode, resp.tcaseIds) + this.resolveMarkers(fileResults, projectCode) + this.validateDuplicateMappings(projectCode, fileResults, Object.values(resp.targetsBySeq)) + + const createRunResult = await this.createNewRun(projectCode, resp.tcaseIds) + runId = createRunResult.runId + this.skipUploaderDuplicateValidation = !createRunResult.reusedExisting } const results = fileResults.flatMap((fileResult) => fileResult.results) @@ -166,7 +174,10 @@ export class ResultUploadCommandHandler { protected detectProjectCodeFromTCaseNames(fileResults: FileResults[]) { for (const { results } of fileResults) { for (const result of results) { - if (result.name) { + if (result.markerResolution === 'resolved' && result.marker?.projectCode) { + return result.marker.projectCode + } + if (result.markerResolution !== 'resolved-none' && result.name) { const code = this.markerParser.detectProjectCode(result.name) if (code) return code } @@ -194,7 +205,8 @@ export class ResultUploadCommandHandler { continue } - const seq = this.markerParser.extractSeq(result.name, projectCode) + const marker = this.resolveMarker(result, projectCode) + const seq = marker?.seq ?? null resultsWithSeqAndFile.push({ seq, file, @@ -208,7 +220,7 @@ export class ResultUploadCommandHandler { } // Now fetch the test cases by their sequence numbers - const apiTCasesMap: Record = {} + const apiTCasesMap: Record = {} if (seqIdsSet.size > 0) { const tcaseMarkers = Array.from(seqIdsSet).map((v) => this.markerParser.formatMarker(projectCode, v) @@ -222,7 +234,11 @@ export class ResultUploadCommandHandler { }) for (const tcase of response.data) { - apiTCasesMap[tcase.seq] = tcase + apiTCasesMap[tcase.seq] = { + id: tcase.id, + seq: tcase.seq, + title: tcase.title, + } } if (response.data.length < DEFAULT_PAGE_SIZE) { @@ -261,10 +277,22 @@ export class ResultUploadCommandHandler { for (let i = 0; i < keys.length; i++) { const marker = this.markerParser.formatMarker(projectCode, newTCases[i].seq) + const duplicateTargetAllowed = (tcasesToCreateMap[keys[i]] || []).length > 1 for (const result of tcasesToCreateMap[keys[i]] || []) { // Prefix the test case markers for use in ResultUploader. The fileResults array // containing the updated name is returned to the caller result.name = `${marker}: ${result.name}` + result.marker = { + projectCode, + seq: newTCases[i].seq, + } + result.markerResolution = 'resolved' + result.allowDuplicateTarget = duplicateTargetAllowed + } + apiTCasesMap[newTCases[i].seq] = { + id: newTCases[i].id, + seq: newTCases[i].seq, + title: keys[i], } tcaseIds.push(newTCases[i].id) } @@ -274,7 +302,7 @@ export class ResultUploadCommandHandler { return printErrorThenExit('No valid test cases found in any of the files') } - return { tcaseIds, fileResults } + return { tcaseIds, fileResults, targetsBySeq: apiTCasesMap } } private async createNewTCases(projectCode: string, tcasesToCreate: string[]) { @@ -417,7 +445,7 @@ export class ResultUploadCommandHandler { console.log( chalk.blue(`Test run URL: ${this.baseUrl}/project/${projectCode}/run/${response.id}`) ) - return response.id + return { runId: response.id, reusedExisting: false } } catch (error) { // Check if the error is about conflicting run ID const errorMessage = error instanceof Error ? error.message : String(error) @@ -426,7 +454,7 @@ export class ResultUploadCommandHandler { if (conflictMatch) { const existingRunId = Number(conflictMatch[1]) console.log(chalk.yellow(`Reusing existing test run "${title}" with ID: ${existingRunId}`)) - return existingRunId + return { runId: existingRunId, reusedExisting: true } } // If it's not a conflicting run ID error, re-throw the original error @@ -446,7 +474,83 @@ export class ResultUploadCommandHandler { runFailureLogs: string }) { const runUrl = `${this.baseUrl}/project/${projectCode}/run/${runId}` - const uploader = new ResultUploader(this.markerParser, this.type, { ...this.args, runUrl }) + const uploader = new ResultUploader( + this.type, + { ...this.args, runUrl }, + { skipDuplicateValidation: this.skipUploaderDuplicateValidation } + ) await uploader.handle(results, runFailureLogs) } + + private resolveMarkers(fileResults: FileResults[], projectCode: string) { + for (const { results } of fileResults) { + for (const result of results) { + this.resolveMarker(result, projectCode) + } + } + } + + private validateDuplicateMappings( + projectCode: string, + fileResults: FileResults[], + targets: TCaseTarget[] + ) { + const { duplicates } = mapResolvedResultsToTCases( + projectCode, + fileResults.flatMap((fileResult) => fileResult.results), + targets + ) + + if (!duplicates.length) { + return + } + + this.printDuplicateMappings(projectCode, duplicates) + if (!this.args.force) { + process.exit(1) + } + } + + private resolveMarker(result: TestCaseResult, projectCode: string): TestCaseMarker | null { + if (result.markerResolution === 'resolved' && result.marker) { + return result.marker.projectCode.toLowerCase() === projectCode.toLowerCase() + ? result.marker + : null + } + + if (result.markerResolution === 'resolved-none') { + return null + } + + if (!result.name) { + result.markerResolution = 'resolved-none' + return null + } + + const seq = this.markerParser.extractSeq(result.name, projectCode) + if (seq === null) { + result.markerResolution = 'resolved-none' + return null + } + + result.marker = { + projectCode: this.markerParser.detectProjectCode(result.name) || projectCode, + seq, + } + result.markerResolution = 'resolved' + return result.marker + } + + private printDuplicateMappings(projectCode: string, duplicates: DuplicateTCaseMapping[]) { + const header = this.args.force ? chalk.yellow('Warning:') : chalk.red('Error:') + for (const duplicate of duplicates) { + console.error( + `${header} multiple results map to ${chalk.green(`${projectCode}-${duplicate.tcase.seq}`)} (${chalk.blue(duplicate.tcase.title)}):` + ) + for (const result of duplicate.results) { + const folderMessage = result.folder ? ` "${result.folder}" ->` : '' + console.error(` -${folderMessage} "${result.name}"`) + } + } + } } diff --git a/src/utils/result-upload/ResultUploader.ts b/src/utils/result-upload/ResultUploader.ts index e32a36b..297c67c 100644 --- a/src/utils/result-upload/ResultUploader.ts +++ b/src/utils/result-upload/ResultUploader.ts @@ -6,7 +6,7 @@ import { parseRunUrl, printError, printErrorThenExit, twirlLoader } from '../mis import { Api, createApi } from '../../api' import { Attachment, TestCaseResult } from './types' import { ResultUploadCommandArgs, UploadCommandType } from './ResultUploadCommandHandler' -import type { MarkerParser } from './MarkerParser' +import { DuplicateTCaseMapping, TCaseWithResult, mapResolvedResultsToTCases } from './mapping' const MAX_CONCURRENT_BATCH_UPLOADS = 3 const MAX_BATCH_SIZE_BYTES = 100 * 1024 * 1024 // 100 MiB @@ -19,9 +19,9 @@ export class ResultUploader { private run: number constructor( - private markerParser: MarkerParser, private type: UploadCommandType, - private args: Arguments + private args: Arguments, + private options: { skipDuplicateValidation?: boolean } = {} ) { const apiToken = process.env.QAS_TOKEN! const { url, project, run } = parseRunUrl(args) @@ -34,8 +34,11 @@ export class ResultUploader { async handle(results: TestCaseResult[], runFailureLogs?: string) { const tcases = await this.api.runs.getTCases(this.project, this.run).catch(printErrorThenExit) - const { results: mappedResults, missing } = this.mapTestCaseResults(results, tcases) + const { results: mappedResults, missing, duplicates } = this.mapTestCaseResults(results, tcases) this.validateAndPrintMissingTestCases(missing) + if (!this.options.skipDuplicateValidation) { + this.validateAndPrintDuplicateMappings(duplicates) + } this.validateAndPrintMissingAttachments(mappedResults) console.log( @@ -88,6 +91,27 @@ export class ResultUploader { console.log(chalk.dim(`\nSkipped ${count} unmatched test${count === 1 ? '' : 's'}`)) } + private validateAndPrintDuplicateMappings(duplicates: DuplicateTCaseMapping[]) { + if (!duplicates.length) { + return + } + + const header = this.args.force ? chalk.yellow('Warning:') : chalk.red('Error:') + for (const duplicate of duplicates) { + console.error( + `${header} multiple results map to ${chalk.green(`${this.project}-${duplicate.tcase.seq}`)} (${chalk.blue(duplicate.tcase.title)}):` + ) + for (const result of duplicate.results) { + const folderMessage = result.folder ? ` "${result.folder}" ->` : '' + console.error(` -${folderMessage} "${result.name}"`) + } + } + + if (!this.args.force) { + process.exit(1) + } + } + private printMissingTestCaseGuidance(missing: TestCaseResult[]) { if (this.type === 'junit-upload') { this.printJUnitGuidance() @@ -371,24 +395,7 @@ ${chalk.yellow('To fix this issue, choose one of the following options:')} } private mapTestCaseResults = (testcaseResults: TestCaseResult[], testcases: RunTCase[]) => { - const results: TCaseWithResult[] = [] - const missing: TestCaseResult[] = [] - - testcaseResults.forEach((result) => { - if (result.name) { - const tcase = testcases.find((tcase) => - this.markerParser.nameMatchesTCase(result.name, this.project, tcase.seq) - ) - - if (tcase) { - results.push({ result, tcase }) - return - } - } - missing.push(result) - }) - - return { results, missing } + return mapResolvedResultsToTCases(this.project, testcaseResults, testcases) } } @@ -396,11 +403,6 @@ export const setMaxResultsInRequest = (max: number) => { MAX_RESULTS_IN_REQUEST = max } -interface TCaseWithResult { - tcase: RunTCase - result: TestCaseResult -} - const makeListHtml = (list: { name: string; url: string }[]) => { return `
    ${list.map((item) => `
  • ${escapeHtml(item.name)}
    • `).join('\n')} diff --git a/src/utils/result-upload/mapping.ts b/src/utils/result-upload/mapping.ts new file mode 100644 index 0000000..93a91f7 --- /dev/null +++ b/src/utils/result-upload/mapping.ts @@ -0,0 +1,63 @@ +import { TestCaseResult } from './types' + +export interface TCaseTarget { + id: string + seq: number + title: string +} + +export interface TCaseWithResult { + tcase: T + result: TestCaseResult +} + +export interface DuplicateTCaseMapping { + tcase: T + results: TestCaseResult[] +} + +export const mapResolvedResultsToTCases = ( + projectCode: string, + testcaseResults: TestCaseResult[], + testcases: T[] +) => { + const testcasesBySeq = new Map(testcases.map((tcase) => [tcase.seq, tcase])) + const results: TCaseWithResult[] = [] + const missing: TestCaseResult[] = [] + + testcaseResults.forEach((result) => { + if (result.marker && result.marker.projectCode.toLowerCase() === projectCode.toLowerCase()) { + const tcase = testcasesBySeq.get(result.marker.seq) + if (tcase) { + results.push({ result, tcase }) + return + } + } + missing.push(result) + }) + + return { results, missing, duplicates: findDuplicateTCaseMappings(results) } +} + +export const findDuplicateTCaseMappings = ( + results: TCaseWithResult[] +): DuplicateTCaseMapping[] => { + const duplicates = new Map>() + + for (const item of results) { + const existing = duplicates.get(item.tcase.id) + if (existing) { + existing.results.push(item.result) + continue + } + + duplicates.set(item.tcase.id, { + tcase: item.tcase, + results: [item.result], + }) + } + + return Array.from(duplicates.values()) + .filter((duplicate) => duplicate.results.length > 1) + .filter((duplicate) => duplicate.results.some((result) => !result.allowDuplicateTarget)) +} diff --git a/src/utils/result-upload/parsers/allureParser.ts b/src/utils/result-upload/parsers/allureParser.ts index a53aa61..8a083b6 100644 --- a/src/utils/result-upload/parsers/allureParser.ts +++ b/src/utils/result-upload/parsers/allureParser.ts @@ -5,9 +5,9 @@ import stripAnsi from 'strip-ansi' import z from 'zod' import { ResultStatus } from '../../../api/schemas' import { parseTCaseUrl } from '../../misc' -import { formatMarker, getMarkerFromText } from '../MarkerParser' +import { formatMarker, getParsedMarkerFromText } from '../MarkerParser' import { Parser, ParserOptions } from '../ResultUploadCommandHandler' -import { Attachment, ParseResult, TestCaseResult } from '../types' +import { Attachment, ParseResult, TestCaseMarker, TestCaseResult } from '../types' import { getAttachments } from '../utils' // Allure result file schema reference: @@ -154,7 +154,11 @@ export const parseAllureResults: Parser = async ( const marker = extractMarker(parsedResult) const index = testcases.push({ - name: marker ? `${marker}: ${parsedResult.name}` : parsedResult.name, + name: marker + ? `${formatMarker(marker.projectCode, marker.seq)}: ${parsedResult.name}` + : parsedResult.name, + marker: marker ?? null, + markerResolution: marker ? 'resolved' : 'resolved-none', folder: getFolder(parsedResult), status, message: buildMessage(parsedResult, status, options), @@ -251,23 +255,26 @@ const buildMessage = ( return message } -const extractMarker = (result: AllureResult): string | undefined => { - return getMarkerFromTmsLinks(result.links) || getMarkerFromText(result.name) +const extractMarker = (result: AllureResult): TestCaseMarker | undefined => { + return getMarkerFromTmsLinks(result.links) || getParsedMarkerFromText(result.name) } -const getMarkerFromTmsLinks = (links: AllureResult['links']): string | undefined => { +const getMarkerFromTmsLinks = (links: AllureResult['links']): TestCaseMarker | undefined => { const tmsLinks = (links || []).filter((link) => link.type?.toLowerCase() === 'tms') for (const link of tmsLinks) { if (!link.url) continue const parsed = parseTCaseUrl(link.url) if (parsed) { - return formatMarker(parsed.project, parsed.tcaseSeq) + return { + projectCode: parsed.project, + seq: parsed.tcaseSeq, + } } } for (const link of tmsLinks) { - const markerFromName = getMarkerFromText(link.name) + const markerFromName = getParsedMarkerFromText(link.name) if (markerFromName) { return markerFromName } diff --git a/src/utils/result-upload/parsers/junitXmlParser.ts b/src/utils/result-upload/parsers/junitXmlParser.ts index 30b634c..5686adf 100644 --- a/src/utils/result-upload/parsers/junitXmlParser.ts +++ b/src/utils/result-upload/parsers/junitXmlParser.ts @@ -1,6 +1,7 @@ import escapeHtml from 'escape-html' import xml from 'xml2js' import z from 'zod' +import { getParsedMarkerFromText } from '../MarkerParser' import { Attachment, ParseResult, TestCaseResult } from '../types' import { Parser, ParserOptions } from '../ResultUploadCommandHandler' import { ResultStatus } from '../../../api/schemas' @@ -148,11 +149,14 @@ export const parseJUnitXml: Parser = async ( // generic suite (e.g., "pytest"). For runners where classname matches the // suite name (e.g., Playwright), this produces the same result. const folder = tcase.$.classname ?? suite.$?.name ?? '' + const parsedMarker = getParsedMarkerFromText(tcaseName) const index = testcases.push({ ...result, folder, name: tcaseName, + marker: parsedMarker ?? null, + markerResolution: parsedMarker ? 'resolved' : 'needs-project-resolution', timeTaken: Number.isFinite(timeTakenSeconds) && timeTakenSeconds >= 0 ? Math.round(timeTakenSeconds * 1000) diff --git a/src/utils/result-upload/parsers/playwrightJsonParser.ts b/src/utils/result-upload/parsers/playwrightJsonParser.ts index 858fbac..0997787 100644 --- a/src/utils/result-upload/parsers/playwrightJsonParser.ts +++ b/src/utils/result-upload/parsers/playwrightJsonParser.ts @@ -5,8 +5,9 @@ import { Attachment, ParseResult, TestCaseResult } from '../types' import { Parser, ParserOptions } from '../ResultUploadCommandHandler' import { ResultStatus } from '../../../api/schemas' import { parseTCaseUrl } from '../../misc' -import { formatMarker } from '../MarkerParser' +import { formatMarker, getParsedMarkerFromText } from '../MarkerParser' import { getAttachments } from '../utils' +import type { TestCaseMarker } from '../types' // Schema definition as per https://github.com/microsoft/playwright/blob/main/packages/playwright/types/testReporter.d.ts @@ -108,6 +109,7 @@ export const parsePlaywrightJson: Parser = async ( const status = mapPlaywrightStatus(test.status) const message = buildMessage(result, status, options) const baseName = `${titlePrefix}${spec.title}` + const fallbackMarker = getParsedMarkerFromText(spec.title) const attachmentPaths: string[] = [] for (const out of result.attachments || []) { @@ -120,16 +122,30 @@ export const parsePlaywrightJson: Parser = async ( attachmentPaths[0]?.startsWith('/') ? undefined : attachmentBaseDirectory ) - // Fan out: one TestCaseResult per unique annotation, or one with no prefix if no annotations - const uniqueMarkers = [...new Set(markers)] - const resultNames = + // Fan out: one TestCaseResult per unique annotation, or one result using the spec title marker. + const uniqueMarkers = dedupeMarkers(markers) + const resultsToCreate = uniqueMarkers.length > 0 - ? uniqueMarkers.map((marker) => `${marker}: ${baseName}`) - : [baseName] - - for (const name of resultNames) { + ? uniqueMarkers.map((marker) => ({ + name: `${formatMarker(marker.projectCode, marker.seq)}: ${baseName}`, + marker, + markerResolution: 'resolved' as const, + })) + : [ + { + name: baseName, + marker: fallbackMarker ?? null, + markerResolution: fallbackMarker + ? ('resolved' as const) + : ('resolved-none' as const), + }, + ] + + for (const { name, marker, markerResolution } of resultsToCreate) { const numTestcases = testcases.push({ name, + marker, + markerResolution, folder: topLevelSuite, status, message, @@ -173,19 +189,30 @@ export const parsePlaywrightJson: Parser = async ( return { testCaseResults: testcases, runFailureLogs: runFailureLogParts.join('') } } -const getAllTCaseMarkersFromAnnotations = (annotations: Annotation[]): string[] => { - const markers: string[] = [] +const getAllTCaseMarkersFromAnnotations = (annotations: Annotation[]): TestCaseMarker[] => { + const markers: TestCaseMarker[] = [] for (const annotation of annotations) { if (annotation.type.toLowerCase().includes('test case') && annotation.description) { const res = parseTCaseUrl(annotation.description) if (res) { - markers.push(formatMarker(res.project, res.tcaseSeq)) + markers.push({ + projectCode: res.project, + seq: res.tcaseSeq, + }) } } } return markers } +const dedupeMarkers = (markers: TestCaseMarker[]): TestCaseMarker[] => { + const uniqueMarkers = new Map() + for (const marker of markers) { + uniqueMarkers.set(`${marker.projectCode.toLowerCase()}-${marker.seq}`, marker) + } + return Array.from(uniqueMarkers.values()) +} + const mapPlaywrightStatus = (status: Status): ResultStatus => { switch (status) { case 'expected': diff --git a/src/utils/result-upload/types.ts b/src/utils/result-upload/types.ts index b2cc2ce..9e574ed 100644 --- a/src/utils/result-upload/types.ts +++ b/src/utils/result-upload/types.ts @@ -7,10 +7,23 @@ export interface Attachment { error: Error | null } +export interface TestCaseMarker { + projectCode: string + seq: number +} + +export type MarkerResolution = 'resolved' | 'resolved-none' | 'needs-project-resolution' + export interface TestCaseResult { // Name of the test case extracted from the report. In case of nested suites, it might contain name of - // parent suites as well. Useful for logging and extracting QA Sphere sequence number for the test case + // parent suites as well. Useful for logging and user-facing diagnostics. name: string + // Explicit marker metadata used for mapping to run test cases. Prefer this over parsing the display name. + marker: TestCaseMarker | null + // Controls whether the handler may perform project-aware fallback resolution. + markerResolution: MarkerResolution + // Set for intentional fan-in cases such as repeated create-tcases titles that should share one target test case. + allowDuplicateTarget?: boolean // Name of the test file (or the suite) to which the test belongs, useful for logging purposes folder: string status: ResultStatus From cacd46081a0f54696e62f91cc07e638e5682fadd Mon Sep 17 00:00:00 2001 From: Satvik Choudhary Date: Mon, 6 Apr 2026 12:38:03 +0530 Subject: [PATCH 2/5] Bump package version to 0.6.1 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1a2117c..d2a3960 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "qas-cli", - "version": "0.5.0", + "version": "0.6.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "qas-cli", - "version": "0.5.0", + "version": "0.6.1", "license": "ISC", "dependencies": { "chalk": "^5.4.1", diff --git a/package.json b/package.json index 2fca9c1..8b9601d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "qas-cli", - "version": "0.6.0", + "version": "0.6.1", "description": "QAS CLI is a command line tool for submitting your automation test results to QA Sphere at https://qasphere.com/", "type": "module", "main": "./build/bin/qasphere.js", From dab04d52f49b353cb9d6dd5af4416c25131769a2 Mon Sep 17 00:00:00 2001 From: Satvik Choudhary Date: Mon, 6 Apr 2026 12:54:27 +0530 Subject: [PATCH 3/5] Prevent duplicate mapping failures from creating test cases --- src/tests/result-upload.spec.ts | 26 +++ .../ResultUploadCommandHandler.ts | 173 +++++++++++++----- 2 files changed, 154 insertions(+), 45 deletions(-) diff --git a/src/tests/result-upload.spec.ts b/src/tests/result-upload.spec.ts index a7ccb5c..e038a76 100644 --- a/src/tests/result-upload.spec.ts +++ b/src/tests/result-upload.spec.ts @@ -734,6 +734,32 @@ describe('Marker-based result mapping', () => { expect(numResultUploadCalls()).toBe(0) }) + test('Should fail duplicate target mappings before creating new test cases', async () => { + const numCreateTCasesCalls = countCreateTCasesApiCalls() + const numCreateRunCalls = countCreateRunApiCalls() + const numResultUploadCalls = countResultUploadApiCalls() + createTCasesResponse = { + tcases: [ + { id: '6', seq: 6 }, + { id: '7', seq: 7 }, + ], + } + + try { + await expect( + run( + `junit-upload --project-code ${projectCode} --create-tcases ./src/tests/fixtures/junit-xml/without-markers.xml ./src/tests/fixtures/junit-xml/matching-tcases.xml ./src/tests/fixtures/junit-xml/matching-tcases.xml` + ) + ).rejects.toThrowError() + } finally { + createTCasesResponse = null + } + + expect(numCreateTCasesCalls()).toBe(0) + expect(numCreateRunCalls()).toBe(0) + expect(numResultUploadCalls()).toBe(0) + }) + test('Should allow duplicate target mappings with --force', async () => { const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) const numResultUploadCalls = countResultUploadApiCalls() diff --git a/src/utils/result-upload/ResultUploadCommandHandler.ts b/src/utils/result-upload/ResultUploadCommandHandler.ts index d0df01a..8a2ce09 100644 --- a/src/utils/result-upload/ResultUploadCommandHandler.ts +++ b/src/utils/result-upload/ResultUploadCommandHandler.ts @@ -62,6 +62,8 @@ interface TestCaseResultWithSeqAndFile { result: TestCaseResult } +type PendingTCaseCreations = Record + const DEFAULT_PAGE_SIZE = 5000 export const DEFAULT_FOLDER_TITLE = 'cli-import' const DEFAULT_TCASE_TAGS = ['cli-import'] @@ -132,6 +134,12 @@ export class ResultUploadCommandHandler { fileResults = resp.fileResults this.resolveMarkers(fileResults, projectCode) this.validateDuplicateMappings(projectCode, fileResults, Object.values(resp.targetsBySeq)) + await this.finalizePendingTCases( + projectCode, + resp.pendingTCasesToCreate, + resp.targetsBySeq, + resp.tcaseIds + ) const createRunResult = await this.createNewRun(projectCode, resp.tcaseIds) runId = createRunResult.runId @@ -249,7 +257,7 @@ export class ResultUploadCommandHandler { // Now validate that the test cases with found sequence numbers actually exist const tcaseIds: string[] = [] - const tcasesToCreateMap: Record = {} + const tcasesToCreateMap: PendingTCaseCreations = {} for (const { seq, file, result } of resultsWithSeqAndFile) { if (seq && apiTCasesMap[seq]) { tcaseIds.push(apiTCasesMap[seq].id) @@ -270,44 +278,105 @@ export class ResultUploadCommandHandler { } } - // Create new test cases, if same is requested - if (Object.keys(tcasesToCreateMap).length > 0) { - const keys = Object.keys(tcasesToCreateMap) - const newTCases = await this.createNewTCases(projectCode, keys) - - for (let i = 0; i < keys.length; i++) { - const marker = this.markerParser.formatMarker(projectCode, newTCases[i].seq) - const duplicateTargetAllowed = (tcasesToCreateMap[keys[i]] || []).length > 1 - for (const result of tcasesToCreateMap[keys[i]] || []) { - // Prefix the test case markers for use in ResultUploader. The fileResults array - // containing the updated name is returned to the caller - result.name = `${marker}: ${result.name}` - result.marker = { - projectCode, - seq: newTCases[i].seq, - } - result.markerResolution = 'resolved' - result.allowDuplicateTarget = duplicateTargetAllowed - } - apiTCasesMap[newTCases[i].seq] = { - id: newTCases[i].id, - seq: newTCases[i].seq, - title: keys[i], - } - tcaseIds.push(newTCases[i].id) + const pendingTCasesToCreate = await this.planPendingTCasesToCreate( + projectCode, + tcasesToCreateMap, + apiTCasesMap, + tcaseIds + ) + + if ( + tcaseIds.length === 0 && + Object.keys(pendingTCasesToCreate).length === 0 && + !fileResults.some((fr) => fr.runFailureLogs) + ) { + return printErrorThenExit('No valid test cases found in any of the files') + } + + return { tcaseIds, fileResults, targetsBySeq: apiTCasesMap, pendingTCasesToCreate } + } + + private async planPendingTCasesToCreate( + projectCode: string, + tcasesToCreateMap: PendingTCaseCreations, + apiTCasesMap: Record, + tcaseIds: string[] + ) { + if (Object.keys(tcasesToCreateMap).length === 0) { + return {} + } + + const reusableTCases = await this.getReusableTCasesInDefaultFolder( + projectCode, + Object.keys(tcasesToCreateMap) + ) + const pendingTCasesToCreate: PendingTCaseCreations = {} + + for (const [title, results] of Object.entries(tcasesToCreateMap)) { + const reusableTCase = reusableTCases[title] + if (!reusableTCase) { + pendingTCasesToCreate[title] = results + continue + } + + this.assignResolvedTarget(projectCode, reusableTCase, results) + apiTCasesMap[reusableTCase.seq] = { + id: reusableTCase.id, + seq: reusableTCase.seq, + title, } + tcaseIds.push(reusableTCase.id) } - if (tcaseIds.length === 0 && !fileResults.some((fr) => fr.runFailureLogs)) { - return printErrorThenExit('No valid test cases found in any of the files') + return pendingTCasesToCreate + } + + private async finalizePendingTCases( + projectCode: string, + pendingTCasesToCreate: PendingTCaseCreations, + apiTCasesMap: Record, + tcaseIds: string[] + ) { + const titles = Object.keys(pendingTCasesToCreate) + if (titles.length === 0) { + return } - return { tcaseIds, fileResults, targetsBySeq: apiTCasesMap } + const newTCases = await this.createNewTCases(projectCode, titles) + for (let i = 0; i < titles.length; i++) { + const title = titles[i] + const newTCase = newTCases[i] + this.assignResolvedTarget(projectCode, newTCase, pendingTCasesToCreate[title] || []) + apiTCasesMap[newTCase.seq] = { + id: newTCase.id, + seq: newTCase.seq, + title, + } + tcaseIds.push(newTCase.id) + } } - private async createNewTCases(projectCode: string, tcasesToCreate: string[]) { - console.log(chalk.blue(`Creating test cases for results with no test case markers`)) + private assignResolvedTarget( + projectCode: string, + tcase: { id: string; seq: number }, + results: TestCaseResult[] + ) { + const marker = this.markerParser.formatMarker(projectCode, tcase.seq) + const duplicateTargetAllowed = results.length > 1 + for (const result of results) { + // Prefix the test case markers for use in ResultUploader. The fileResults array + // containing the updated name is returned to the caller + result.name = `${marker}: ${result.name}` + result.marker = { + projectCode, + seq: tcase.seq, + } + result.markerResolution = 'resolved' + result.allowDuplicateTarget = duplicateTargetAllowed + } + } + private async getReusableTCasesInDefaultFolder(projectCode: string, tcasesToCreate: string[]) { // First fetch the default folder ID where we are creating new test cases. // Ideally, there shouldn't be the need to fetch more than one page. let defaultFolderId = null @@ -330,26 +399,40 @@ export class ResultUploadCommandHandler { } } - // If the default folder exists, fetch the test cases in it - const apiTCasesMap: Record = {} - if (defaultFolderId) { - for (let page = 1; ; page++) { - const response = await this.api.testCases.getPaginated(projectCode, { - folders: [defaultFolderId], - page, - limit: DEFAULT_PAGE_SIZE, - }) + const reusableTCases: Record = {} + if (!defaultFolderId) { + return reusableTCases + } - for (const tcase of response.data) { - apiTCasesMap[tcase.title] = tcase - } + const pendingTitles = new Set(tcasesToCreate) + for (let page = 1; pendingTitles.size > 0; page++) { + const response = await this.api.testCases.getPaginated(projectCode, { + folders: [defaultFolderId], + page, + limit: DEFAULT_PAGE_SIZE, + }) - if (response.data.length < DEFAULT_PAGE_SIZE) { - break + for (const tcase of response.data) { + if (!pendingTitles.has(tcase.title)) { + continue } + reusableTCases[tcase.title] = tcase + pendingTitles.delete(tcase.title) + } + + if (response.data.length < DEFAULT_PAGE_SIZE) { + break } } + return reusableTCases + } + + private async createNewTCases(projectCode: string, tcasesToCreate: string[]) { + console.log(chalk.blue(`Creating test cases for results with no test case markers`)) + + const apiTCasesMap = await this.getReusableTCasesInDefaultFolder(projectCode, tcasesToCreate) + // Reuse existing test cases with the same title from the default folder const ret: { id: string; seq: number }[] = [] const idxToFill: number[] = [] From 240fb1066b63ca383e78e780ddcf4ee216c4d992 Mon Sep 17 00:00:00 2001 From: Satvik Choudhary Date: Mon, 6 Apr 2026 13:04:22 +0530 Subject: [PATCH 4/5] Remove issue report markdown files --- QAS_CLI_ISSUES_BRIEF.md | 117 --------------------- QAS_CLI_ISSUES_REPORT.md | 221 --------------------------------------- 2 files changed, 338 deletions(-) delete mode 100644 QAS_CLI_ISSUES_BRIEF.md delete mode 100644 QAS_CLI_ISSUES_REPORT.md diff --git a/QAS_CLI_ISSUES_BRIEF.md b/QAS_CLI_ISSUES_BRIEF.md deleted file mode 100644 index cd4e29a..0000000 --- a/QAS_CLI_ISSUES_BRIEF.md +++ /dev/null @@ -1,117 +0,0 @@ -# qas-cli: incorrect Playwright upload mapping to QA Sphere runs - -## Summary - -`qas-cli playwright-json-upload` can report a successful upload while mapping many Playwright results to the wrong QA Sphere test cases. - -In the reproduced upload: - -- Playwright run: `142` tests total, `140 passed`, `2 failed` -- CLI upload output: `Uploaded 139 test cases` -- QA Sphere run summary after upload: only `33 passed`, `1 failed` - -Root cause: - -- the uploader matches hyphenated markers with substring logic, so `QS1-10427` incorrectly matches `QS1-104` -- the Playwright parser only uses the first `test case` annotation and drops additional ones - -## Repro - -Using the Chromium Playwright JSON report from `tms-2` and uploading to existing run `QS1/run/53`: - -- `139` results were considered matched -- but they collapsed onto only `34` unique run test cases - -Observed collisions: - -- `QS1-104` absorbed `58` results -- `QS1-103` absorbed `25` -- `QS1-106` absorbed `19` -- `QS1-105` absorbed `4` - -This produced the exact run summary seen in QA Sphere: `33 passed / 1 failed`. - -## Issues - -### 1. Exact marker matching is broken - -File: - -- [src/utils/result-upload/MarkerParser.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/MarkerParser.ts#L152) - -Current code: - -- `name.toLowerCase().includes(hyphenated.toLowerCase())` - -Problem: - -- `QS1-10427` matches `QS1-104` -- `QS1-10775` matches `QS1-107` - -Expected: - -- hyphenated markers must match exactly with boundaries -- `QS1-104` must not match `QS1-10427` - -### 2. Playwright multi-annotation support is incomplete - -File: - -- [src/utils/result-upload/playwrightJsonParser.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/playwrightJsonParser.ts#L106) - -Problem: - -- only the first QA Sphere `test case` annotation is used -- additional annotations on the same test are dropped - -Evidence from the reproduced report: - -- `138` tests had annotations -- `26` tests had multiple annotations -- examples included tests with `2`, `3`, `4`, `6`, `7`, `8`, and `10` QA Sphere references - -Expected: - -- one Playwright test with multiple QA Sphere annotations should fan out into multiple upload results, one per annotation - -### 3. Upload success messaging is misleading - -File: - -- [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L31) - -Problem: - -- final message says `Uploaded 139 test cases` -- in reality those were `139` mapped result records targeting only `34` unique run test cases -- duplicate mapping collisions are not surfaced - -Expected: - -- output should distinguish: - - number of source results - - number of mapped results - - number of unique target test cases -- uploader should warn or fail when many results collapse onto the same target case unexpectedly - -## Acceptance Criteria - -1. `nameMatchesTCase()` uses exact matching for hyphenated markers and no longer allows prefix collisions. -2. Playwright parsing supports multiple `test case` annotations by generating one upload result per referenced QA Sphere case. -3. Uploader output clearly reports mapped results vs unique target test cases. -4. Regression tests cover: - - `QS1-104` vs `QS1-10427` - - single vs multiple Playwright annotations - - duplicate-mapping detection or reporting - -## Notes - -Three unmatched tests in the reproduced run were genuinely unlinked and are not part of this bug: - -- `should show error for invalid credentials` -- `Complete smoke test of basic functionality` -- `should show no run cards for project with no assigned runs` - -Full report: - -- [QAS_CLI_ISSUES_REPORT.md](/Users/apple/Github/Hypersequent/qas-cli/QAS_CLI_ISSUES_REPORT.md) diff --git a/QAS_CLI_ISSUES_REPORT.md b/QAS_CLI_ISSUES_REPORT.md deleted file mode 100644 index ef6dabc..0000000 --- a/QAS_CLI_ISSUES_REPORT.md +++ /dev/null @@ -1,221 +0,0 @@ -# qas-cli Issue Report - -Date: 2026-03-20 - -## Scope - -This report covers issues found while uploading the Chromium Playwright e2e run from `tms-2` into an existing QA Sphere run with `qas-cli playwright-json-upload`. - -Observed local run summary: - -- `142` Playwright tests total -- `140 passed` -- `2 failed` -- `3` tests had no QA Sphere marker/annotation and were intentionally unmatched - -Observed uploader output: - -- first upload failed on the 3 unmatched tests -- retry with `--ignore-unmatched` reported `Uploaded 139 test cases` - -Observed QA Sphere run summary after upload: - -- `33 passed` -- `1 failed` -- `762 open` - -That mismatch is explained by the issues below. - -## Findings - -### 1. Critical: `nameMatchesTCase()` uses substring matching for hyphenated markers - -Files: - -- [src/utils/result-upload/MarkerParser.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/MarkerParser.ts#L152) -- [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L322) - -Current behavior: - -- `nameMatchesTCase()` builds a canonical marker like `QS1-104` -- it then checks `name.toLowerCase().includes(hyphenated.toLowerCase())` - -That is incorrect for hyphenated markers because: - -- `QS1-10427` incorrectly matches `QS1-104` -- `QS1-10775` incorrectly matches `QS1-107` -- any shorter prefix test case already present in the run can absorb unrelated longer markers - -### Evidence - -Replaying the current uploader logic against the generated Playwright JSON and the existing run's test cases produced: - -- `142` total Playwright results -- `139` considered matched -- only `34` unique run test cases targeted - -The matched `seq` set was: - -- `8, 13, 15, 17, 19, 21, 32, 33, 34, 35, 36, 37, 38, 39, 103, 104, 105, 106, 107, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 223, 365` - -The worst collisions were: - -- `QS1-104` matched `58` uploaded results -- `QS1-103` matched `25` -- `QS1-106` matched `19` -- `QS1-105` matched `4` - -This exactly explains why the run summary only showed `33 passed / 1 failed`: the results were collapsed onto a small set of prefix case IDs already present in the run. - -### Impact - -- Results are uploaded to the wrong test cases -- Existing run summaries become misleading -- Failures can be assigned to unrelated test cases -- The CLI can appear to succeed while corrupting run-level reporting - -### Suggested fix - -Replace substring matching with exact marker matching using boundaries. - -Expected semantics: - -- `QS1-104` should match only `QS1-104` -- it must not match `QS1-10427` - -Reasonable implementation options: - -- use a regex with boundaries around the full marker -- or parse markers from the result name first, then compare normalized marker values exactly - -Suggested tests: - -- `nameMatchesTCase('QS1-10427: some test', 'QS1', 104)` should be `false` -- `nameMatchesTCase('QS1-104: some test', 'QS1', 104)` should be `true` -- `nameMatchesTCase('some test (QS1-10427)', 'QS1', 10427)` should be `true` -- `nameMatchesTCase('some test (QS1-10427)', 'QS1', 104)` should be `false` - -### 2. High: Playwright parser only uses the first `test case` annotation - -Files: - -- [src/utils/result-upload/playwrightJsonParser.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/playwrightJsonParser.ts#L106) -- [src/utils/result-upload/playwrightJsonParser.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/playwrightJsonParser.ts#L159) - -Current behavior: - -- `getTCaseMarkerFromAnnotations()` returns only a single marker -- `parsePlaywrightJson()` prefixes that one marker into the result name -- any additional QA Sphere annotations on the same Playwright test are discarded - -### Evidence - -In the generated Playwright JSON: - -- `138` tests had at least one annotation -- `26` tests had multiple QA Sphere annotations - -Examples: - -- one test had `7` annotations: `QS1-10427`, `QS1-10428`, `QS1-10429`, `QS1-10430`, `QS1-10668`, `QS1-10669`, `QS1-10670` -- another had `8` annotations: `QS1-10591` through `QS1-10598` -- another had `6` annotations: `QS1-10775` through `QS1-10780` - -Today only the first annotation is used, so the rest are lost before matching even starts. - -### Impact - -- Multi-case Playwright tests cannot upload all referenced QA Sphere cases -- Uploaded coverage is incomplete even if marker matching is fixed -- Complex scenario tests appear under only one case instead of all declared cases - -### Suggested fix - -Change the Playwright parser to return multiple `TestCaseResult` entries when a test has multiple QA Sphere annotations. - -Expected behavior: - -- one Playwright test with `N` QA Sphere annotations should fan out into `N` result entries -- each entry should carry the same status, attachments, comment, and timing -- each entry should map to its own QA Sphere test case - -Suggested tests: - -- single annotation still produces one result -- multiple annotations produce multiple results -- annotation order does not drop later markers -- annotations should take precedence over name markers when both exist - -### 3. Medium: uploader messaging is misleading and hides destructive fan-in - -Files: - -- [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L31) -- [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L272) -- [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L322) - -Current behavior: - -- uploader logs `Uploaded ${mappedResults.length} test cases` -- in this repro that message was `Uploaded 139 test cases` -- but those `139` mapped results collapsed onto only `34` unique run test cases because of issue 1 - -The message is misleading even aside from the bug: - -- it is counting uploaded result records, not distinct test cases -- it does not report duplicate mappings -- it does not warn when many source results fan into a small set of target test cases - -### Impact - -- false confidence after upload -- hard to spot mapping corruption -- user sees success in CLI but wrong numbers in QA Sphere UI - -### Suggested fix - -Improve diagnostics before and after upload: - -- log both `mapped results` and `unique target test cases` -- if multiple source results map to the same run test case, warn or fail unless explicitly allowed -- change final message to `Uploaded X results to Y test cases` - -Also consider validating API responses more strictly: - -- compare requested batch size with returned ID count -- surface partial-acceptance conditions clearly - -Suggested tests: - -- duplicate mappings should be visible in output -- final message should distinguish results vs distinct cases - -## Not Bugs - -These were investigated and should not be treated as defects: - -- Uploading to an existing run only maps against test cases already present in that run. - This is by design in [src/utils/result-upload/ResultUploader.ts](/Users/apple/Github/Hypersequent/qas-cli/src/utils/result-upload/ResultUploader.ts#L32). - -- The 3 unmatched Playwright tests in this run were genuinely unlinked: - - `should show error for invalid credentials` - - `Complete smoke test of basic functionality` - - `should show no run cards for project with no assigned runs` - -## Recommended Fix Order - -1. Fix exact marker matching in `nameMatchesTCase()` -2. Add regression tests for prefix collisions -3. Fix Playwright multi-annotation fan-out -4. Add regression tests for multi-annotation parsing and upload mapping -5. Improve uploader diagnostics and final reporting - -## Minimal Repro Summary - -Using the generated Playwright JSON from the Chromium run: - -- the current parser/uploader mapped `139` results -- but only `34` unique run test cases were targeted -- most high-numbered `QS1-*` markers were incorrectly absorbed by short prefix IDs like `QS1-103`, `QS1-104`, `QS1-105`, `QS1-106`, `QS1-107` - -That is the main reason the QA Sphere UI showed only `33 passed / 1 failed` instead of reflecting the broader Chromium run. From c8094a556ec4e72d8599129bd1a2297f15428fa2 Mon Sep 17 00:00:00 2001 From: Satvik Choudhary Date: Fri, 10 Apr 2026 21:51:39 +0530 Subject: [PATCH 5/5] Bump package version to 0.7.0 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index d2a3960..4145b10 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "qas-cli", - "version": "0.6.1", + "version": "0.7.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "qas-cli", - "version": "0.6.1", + "version": "0.7.0", "license": "ISC", "dependencies": { "chalk": "^5.4.1", diff --git a/package.json b/package.json index 8b9601d..58f2ed5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "qas-cli", - "version": "0.6.1", + "version": "0.7.0", "description": "QAS CLI is a command line tool for submitting your automation test results to QA Sphere at https://qasphere.com/", "type": "module", "main": "./build/bin/qasphere.js",