-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
fix: re-enable cypress checks #32008
Conversation
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.
I've completed my review and didn't find any issues.
Need a new review? Comment
/korbit-review
on this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-review
command in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-description
command in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolve
command in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Current Korbit Configuration
General Settings
Setting Value Review Schedule Automatic excluding drafts Max Issue Count 10 Automatic PR Descriptions ❌ Issue Categories
Category Enabled Naming ✅ Database Operations ✅ Documentation ✅ Logging ✅ Error Handling ✅ Systems and Environment ✅ Objects and Data Structures ✅ Readability and Maintainability ✅ Asynchronous Processing ✅ Design Patterns ✅ Third-Party Libraries ✅ Performance ✅ Security ✅ Functionality ✅ Feedback and Support
Note
Korbit Pro is free for open source projects 🎉
Looking to add Korbit to your team? Get started with a free 2 week trial here
I got a bit confused and reckless on #31858, when I disabled many cypress tests, many of which are or may be flaky at times, but in this instance were probably triggered by some real issues in that PR. Renabling them all and will monitor PRs and master merges to re-disable the ones flaking as needed
1107221
to
e30a2e4
Compare
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.
LGTM
@mistercrunch This seems to have broken CI. Should we revert? ![]() |
Oh yes, this is due to checks being different on I little while back we decided to only use the Cypress All this started when we unknowingly accumulated significant costs related to our high usage last year, and decided to try and shut down the bulk of the load. |
This reverts commit b12f515.
Revert is here -> #32045 |
I got a bit confused and reckless on #31858 while dealing with a sizable merge conflict, proceeded to disable many cypress tests, many of which are or may be flaky at times, but in this instance were probably triggered by some real issues in that PR.
Renabling them all and will monitor PRs and master merges to re-disable the ones flaking as needed.
Note that prior to the PR issues I created on #31858, there were all sorts of cypress issues that we couldn't pinpoint to any particular merges, and that led a few of us to make to call to "disable all flaking tests" with the intent to fix or replace with unit tests:
![Screenshot 2025-01-27 at 9 41 17 PM](https://private-user-images.githubusercontent.com/487433/407196529-35e0000a-c455-48d6-a771-00a2331c9570.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkxNjIyMTksIm5iZiI6MTczOTE2MTkxOSwicGF0aCI6Ii80ODc0MzMvNDA3MTk2NTI5LTM1ZTAwMDBhLWM0NTUtNDhkNi1hNzcxLTAwYTIzMzFjOTU3MC5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwMjEwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDIxMFQwNDMxNTlaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT00MzdiYTE3ZDA0YWYzMGNkMzY5ZGVkNTUzNjM4ZmM3M2JhNTMxZDNhODFiMDRiNTJkYzRmZjY4NDFiMmViZGE1JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.dP99OubFhNnkzGtQCS3gO9tneV2i7ILwwqmHAXhd7e8)
Seems there are other new issues with the way we goup/report tests as well. Still needs weeding.