Conversation
Summary of ChangesHello @satvik007, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the QAS CLI's capabilities by integrating native support for Allure test reports. Users can now upload Allure results directories, leveraging a new parser that handles status mapping, folder organization, and attachment resolution. The update also includes enhanced error handling for invalid Allure files and comprehensive documentation, making the CLI more versatile for diverse test automation workflows. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Introduce a directory-based Allure parser and wire it into the upload pipeline so users can upload Allure 2 results with TMS-link or marker-based matching. Extend CLI, tests, and docs for allure-upload, support force-aware skipping of invalid Allure files, and add AGENTS.md as a symlink to CLAUDE.md. Co-authored-by: Cursor <cursoragent@cursor.com>
… and expanded tests - Switch allureParser to use formatMarker() from MarkerParser instead of the removed getTCaseMarker() from misc - Replace inline this.type === 'allure-upload' checks in parseFiles() with a module-level directoryInputTypes Set for cleaner extensibility - Add 7 new allure-parsing unit tests: ANSI stripping, null arrays, HTML escaping, container/non-result file filtering, empty directory, failed test skip behavior, and negative duration handling - Update CLAUDE.md to document directoryInputTypes, allowPartialParse, and allureParser behavior details Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
satvik007
force-pushed
the
feat/allure-upload-support
branch
from
232171f to
71a1ce1
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…erFromText tests - Make links[].url optional in schema so @TMslinK without allure.link.tms.pattern works - Guard getMarkerFromTmsLinks against undefined URL - Add recursive AllureStep schema and collectStepAttachmentPaths for nested attachments - Add getMarkerFromText unit tests for edge cases - Add blank line before getErrorMessage
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Claude finished @satvik007's task in 4m 25s —— View job Review Complete
The latest commit ( No new issues found. Submitted a review with one carried-forward inline comment: the path traversal via Minimized 4 older bot review summaries, deleted 2 stale progress comments. |
|
@codex review |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@Hypersequent/core Please review this PR |
| } | ||
|
|
||
| export type Parser = ( | ||
| // Primary parser input. File-based parsers receive file contents while |
There was a problem hiding this comment.
nit: We can change it to path for file uploads as well. I had done it in xcresults PR - #40 - but that is not merged yet
Not necessary, can be skipped now, and done in future as per requirement
There was a problem hiding this comment.
Where did you get the schema definition/details from ? Can you add links in this file, which might be helpful in future ?
There was a problem hiding this comment.
added links. I got it from docs and also generated from examples repo by using docker containers for different languages/frameworks
…into a single CommandDetails record
|
Claude finished @satvik007's task in 9s —— View job
Review CompleteThe latest commits ( Minimized 6 older bot review summaries, resolved 1 duplicate bot thread. |
There was a problem hiding this comment.
Review Summary
The latest commits (3944626, 09b1f1d) address all remaining reviewer feedback — commandDetails consolidated into a single record per hi-rai's suggestion, and schema reference links added to the test file. The implementation is clean, well-tested, and follows existing codebase patterns.
Issues resolved across PR iterations:
- Marker extraction delegates to shared
getMarkerFromTextfromMarkerParser.ts start/stopfields made optional in Zod schema- Directory validation with user-friendly error message
- CLAUDE.md numbered list formatting restored
- Comprehensive test coverage (empty dir, stop < start, ANSI stripping, HTML escaping, nested step attachments, TMS link edge cases, short markers)
commandDetailsconsolidated into singleRecord<UploadCommandType, CommandDetails>(latest commit)- Schema reference comments added to test file (latest commit)
Still open (1 inline comment, carried forward):
- Path traversal via
attachment.source— ThegetFilefunction inutils.tsusespath.join(basePath, filePath)without validating the resolved path stays within the base directory. This remains the only substantive item to address before merge.
No new issues found in the latest commits.
| const attachmentPaths = [...topLevelPaths, ...stepPaths] | ||
| attachmentsPromises.push({ | ||
| index, | ||
| promise: getAttachments(attachmentPaths, attachmentBaseDirectory || resultsDirectory), |
There was a problem hiding this comment.
Path traversal via attachment.source — still unaddressed (carried forward from prior reviews)
The attachment.source field from Allure JSON is passed directly to getAttachments() → getFile() in utils.ts, which uses path.join(basePath, filePath) without validating the resolved path stays within the base directory. A malicious result file could set source to ../../../../etc/passwd, and path.join would resolve the traversal, allowing arbitrary file reads that get uploaded to QA Sphere.
This is the most relevant for Allure since attachment paths come from user-provided JSON (unlike JUnit/Playwright where paths are typically extracted from test output).
The fix belongs in getFile() in utils.ts (shared by all parsers):
const resolvedPath = path.resolve(basePath, filePath)
if (!resolvedPath.startsWith(path.resolve(basePath) + path.sep) && resolvedPath !== path.resolve(basePath)) {
throw new Error(`Attachment path "${filePath}" escapes the base directory`)
}This was flagged independently by both Claude and Gemini in the first review cycle and remains the only open item.
2 participants
Summary
Adds a new
allure-uploadcommand that ingests Allure 2 results directories and uploads them to QA Sphere.Allure parser (
allureParser.ts)*-result.jsonfiles from a directory; ignores containers, XML, and raw attachmentstype: "tms") or marker in test name (e.g.BD-123)passed → passed,failed → failed,broken → blocked,skipped → skipped,unknown → opensourcefilename within the same directory; also collects attachments from nested steps recursivelysuite/parentSuite/subSuitelabelsformatMarker()fromMarkerParserfor consistent marker formattingallowPartialParsesupport: skips invalid result files instead of aborting when--forceis setCommand wiring
allure-uploadas a third command inmain.tsvia the sameResultUploadCommandModuleallure-uploadtodirectoryInputTypesinResultUploadCommandHandler— the parser receives the directory path rather than file contentsgetMarkerFromTextadded toMarkerParserand exposed in testsTests & fixtures
allure-parsing.spec.ts— 545-line unit test suite covering TMS links, markers, status mapping, suite labels, attachment resolution, nested step attachments, partial parse, invalid files, and edge casesresult-upload.spec.ts— integration tests for the full allure upload flow via MSWmatching-tcases,missing-tcases,empty-tsuite,invalid-results,missing-attachments,without-markersDocumentation
allure-uploadusage, options, and examplesTesting
npm test— all unit and integration tests passnpm run check— typecheck + lint + formatResolves: https://github.com/Hypersequent/tms-issues/issues/2329