-
Notifications
You must be signed in to change notification settings - Fork 712
feat(android): aggregate reports when running more than one test (#1163) #1267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(android): aggregate reports when running more than one test (#1163) #1267
Conversation
✅ Deploy Preview for midscene ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…, remove agent.teardownTestAgent, add one case in android project to show the usage of ReportMergingTool
tangjialei seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
… reports correctly
… for removing getHtmlScripts funtion. Move android test into tests/ai/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements report aggregation functionality for Android tests when running multiple tests with vitest. The feature allows merging individual test reports into a single consolidated report.
- Adds a new
ReportMergingTool
class to handle report aggregation operations - Updates core utilities to support report merging without template inclusion
- Creates test coverage for the report merging functionality
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
packages/core/src/report.ts |
New ReportMergingTool class for aggregating multiple test reports |
packages/core/src/types.ts |
Adds type definitions for test status and report attributes |
packages/core/src/utils.ts |
Exports utilities and adds withTpl parameter to reportHTMLContent |
packages/core/tests/unit-test/report.test.ts |
Comprehensive test suite for report merging functionality |
packages/core/package.json |
Adds export entry for the new report module |
packages/android/tests/ai/merge-reports.test.ts |
Example usage of report merging in Android tests |
packages/android/vitest.config.ts |
Disables parallel file execution for proper report aggregation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
return expectedContents; | ||
} | ||
|
||
describe('repotMergingTool', () => { |
Copilot
AI
Oct 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'repotMergingTool' to 'reportMergingTool'.
describe('repotMergingTool', () => { | |
describe('reportMergingTool', () => { |
Copilot uses AI. Check for mistakes.
if (type !== 'dump') { | ||
// do not write dump file any more | ||
writeFileSync(filePath, fileContent); | ||
writeFileSync(filePath, JSON.stringify(fileContent)); |
Copilot
AI
Oct 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change breaks compatibility for non-JSON content. The function should only stringify when fileContent is an object, not when it's already a string.
writeFileSync(filePath, JSON.stringify(fileContent)); | |
if (typeof fileContent === 'string') { | |
writeFileSync(filePath, fileContent); | |
} else { | |
writeFileSync(filePath, JSON.stringify(fileContent)); | |
} |
Copilot uses AI. Check for mistakes.
} | ||
|
||
public mergeReports( | ||
reportFileName: 'AUTO' | string = 'AUTO',// user custom report filename, save into midscene report dir if undefined |
Copilot
AI
Oct 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after comma in inline comment. Should be 'AUTO',// ' -> 'AUTO', // '
reportFileName: 'AUTO' | string = 'AUTO',// user custom report filename, save into midscene report dir if undefined | |
reportFileName: 'AUTO' | string = 'AUTO', // user custom report filename, save into midscene report dir if undefined |
Copilot uses AI. Check for mistakes.
playwright_test_id: reportAttributes.testId, | ||
playwright_test_description: reportAttributes.testDescription, | ||
}, | ||
}, undefined, undefined, false) + '\n';// use existed function to achieve report script content |
Copilot
AI
Oct 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space before inline comment and grammatical error. Should be 'false) + '\n'; // use existing function to generate report script content'
}, undefined, undefined, false) + '\n';// use existed function to achieve report script content | |
}, undefined, undefined, false) + '\n'; // use existing function to generate report script content |
Copilot uses AI. Check for mistakes.
testId: `${ctx.task.name}`, //ID is a unique identifier used by the front end to distinguish each use case! | ||
testTitle: `${ctx.task.name}`, | ||
testDescription: 'description', | ||
testDuration: (Date.now() - ctx.task.result?.startTime!) | 0, |
Copilot
AI
Oct 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using bitwise OR with 0 to convert to integer is unclear. Use Math.floor() or parseInt() for better readability and intent.
testDuration: (Date.now() - ctx.task.result?.startTime!) | 0, | |
testDuration: Math.floor(Date.now() - ctx.task.result?.startTime!), |
Copilot uses AI. Check for mistakes.
86a7b9c
into
web-infra-dev:feat/report-merging-tool
aggregate reports when running multiple android tests using vitest.