-
Notifications
You must be signed in to change notification settings - Fork 13k
chore: qase integration #37538
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
chore: qase integration #37538
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughThe changes add Qase TestOps integration to the federation matrix test suite by configuring Jest reporters, setting up environment variables, adding a Jest setup file for hierarchical test registration, installing the jest-qase-reporter dependency, and removing debug console.log statements from test files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@ggazzo I'll probably be off for the next few days. Can you add the qase jest api token to the secret |
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
ee/packages/federation-matrix/tests/setup-qase.ts (1)
54-70: Improve type safety and async handling.The wrapper uses
anytypes and always wraps test functions in async wrappers, which could cause issues with synchronous tests or affect type checking.Apply this diff to improve type safety and preserve async/sync behavior:
- global.it = ((testName: any, fn?: any, timeout?: number) => { + global.it = ((testName: string | number, fn?: jest.ProvidesCallback, timeout?: number) => { // Handle qase-wrapped test names (qase returns a string) if (typeof testName === 'string' && fn) { return currentIt( testName, - async () => { - // Set suite immediately at the start of the test - qase.suite(currentPath); - // Call the original test function and return the result - return fn(); - }, + function (this: any, ...args: any[]) { + qase.suite(currentPath); + return fn.apply(this, args); + }, timeout, ); } // Handle cases where testName might be a number or other type return currentIt(testName, fn, timeout); }) as typeof global.it;This preserves the original function's async/sync behavior and maintains proper
thisbinding.ee/packages/federation-matrix/jest.config.federation.ts (1)
28-46: Make debug conditional to reduce CI log verbosity.The reporters configuration is correctly structured and matches the jest-qase-reporter v2.1.3 API. All options used—
mode: 'testops',testops.api.token,testops.project, andtestops.run.complete—are valid according to v2.1.3 specifications. However,debug: truewill produce verbose output in CI logs. Consider making it conditional:debug: true, + debug: process.env.CI !== 'true',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
.github/workflows/ci.yml(1 hunks)ee/packages/federation-matrix/jest.config.federation.ts(1 hunks)ee/packages/federation-matrix/tests/end-to-end/room.spec.ts(0 hunks)ee/packages/federation-matrix/tests/helper/ddp-listener.ts(0 hunks)ee/packages/federation-matrix/tests/setup-qase.ts(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (2)
- ee/packages/federation-matrix/tests/helper/ddp-listener.ts
- ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
🔇 Additional comments (4)
.github/workflows/ci.yml (1)
657-657: LGTM!The environment variable is correctly passed from secrets to enable Qase integration for federation tests.
ee/packages/federation-matrix/jest.config.federation.ts (1)
24-25: LGTM!The setupFilesAfterEnv configuration correctly enables Qase integration via the new setup file. The comment clearly documents how to disable the integration.
ee/packages/federation-matrix/tests/setup-qase.ts (1)
1-107: Well-structured approach to Qase integration.The monkey-patching strategy effectively addresses the test suite hierarchy reporting issue without modifying individual test files. The conditional activation and clear documentation make it easy to understand and disable if needed.
package.json (1)
24-24: No issues found with jest-qase-reporter version 2.1.3.The package version is the latest available release and has no known security vulnerabilities according to Snyk and GitHub Advisories.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## test/federation-media-messages #37538 +/- ##
=================================================================
Coverage ? 69.03%
=================================================================
Files ? 3358
Lines ? 114228
Branches ? 20569
=================================================================
Hits ? 78852
Misses ? 33288
Partials ? 2088
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
This PR integrates federation matrix integration tests in our Qase test suite. Simply using
jest-qase-reporterwithout any manual work caused our tests directory structure to be reported as well, not following the standard of test suite structure we currently have in Qase. The solution for that was to create a wrapper for describe/it/test that calledqase.suite()internally.In order to not monkey patch jest functions I could've called
qase.suiteinside of the tests, but this would have made our tests tightly coupled to qase, making it difficult to remove it if we ever decided to move to a different tool. What I did instead was to monkey patch these test functions inside of a jest setup config in a way that you just have to stop loading that config injest.config.federation.tsand the integration with Qase is gone.In order to get the pipeline to pass we need to create a production jest qase api token and add a secret to the github pipeline called
QASE_TESTOPS_JEST_API_TOKEN.Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Tests
Chores