-
Notifications
You must be signed in to change notification settings - Fork 233
test: cleanup common issues #938
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
base: main
Are you sure you want to change the base?
Conversation
@Udit-takkar is attempting to deploy a commit to the Antiwork Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a new E2E test utility to delete common issue records by title and updates the common issues E2E spec to call this cleanup after specific tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant CleanupHelper
participant DB
Test->>CleanupHelper: deleteCommonIssuesFromDb([titles])
CleanupHelper->>DB: DELETE FROM issueGroups WHERE title IN (titles)
DB-->>CleanupHelper: result / error
CleanupHelper-->>Test: void
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Signed-off-by: Udit Takkar <[email protected]>
@binary-koan can I get a review on this PR? and others related to cleaning up data |
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: 3
♻️ Duplicate comments (1)
tests/e2e/common-issues/commonIssues.spec.ts (1)
2-2
: Resolved: missing import addedImport for the cleanup helper is correctly added. This addresses the earlier review feedback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
tests/e2e/common-issues/commonIssues.spec.ts
(4 hunks)tests/e2e/utils/commonIssueCleanup.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/e2e/utils/commonIssueCleanup.ts (1)
db/schema/issueGroups.ts (1)
issueGroups
(6-19)
tests/e2e/common-issues/commonIssues.spec.ts (1)
tests/e2e/utils/commonIssueCleanup.ts (1)
deleteCommonIssuesFromDb
(5-11)
🔇 Additional comments (2)
tests/e2e/common-issues/commonIssues.spec.ts (2)
78-78
: No-op whitespace changeNothing to do here.
116-116
: LGTM: thorough cleanup after search testAll created issues are cleaned up; this should keep the DB deterministic for downstream tests.
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: 0
♻️ Duplicate comments (1)
tests/e2e/common-issues/commonIssues.spec.ts (1)
82-82
: Also include originalTitle in cleanup to cover mid-test failuresIf the test fails before the rename completes, the record with originalTitle can be left behind. Deleting both titles makes the cleanup resilient.
Apply this diff:
- await deleteCommonIssuesFromDb([newTitle]); + await deleteCommonIssuesFromDb([originalTitle, newTitle]);
🧹 Nitpick comments (1)
tests/e2e/common-issues/commonIssues.spec.ts (1)
59-59
: Guarantee cleanup with try/finally (or afterEach) to avoid residue on assertion failuresRight now, cleanup runs only if the test reaches the end. Wrap test bodies in try/finally (or use test.afterEach for this suite) so DB cleanup always executes even when assertions throw.
Example pattern:
test("should create new common issues with form validation", async ({ page }) => { const createdTitles: string[] = []; try { const titleOnlyIssue = `Test Issue ${generateRandomString(8)}`; const titleDescriptionIssue = `Test Issue with Description ${generateRandomString(8)}`; // ... create issues createdTitles.push(titleOnlyIssue, titleDescriptionIssue); // ... assertions } finally { await deleteCommonIssuesFromDb(createdTitles); } });Optional: ensure the helper logs (at least debug) on failure so silent cleanup errors don’t mask data bleed in CI.
Also applies to: 82-82, 116-116
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/e2e/common-issues/commonIssues.spec.ts
(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/e2e/common-issues/commonIssues.spec.ts (1)
tests/e2e/utils/commonIssueCleanup.ts (1)
deleteCommonIssuesFromDb
(5-11)
🔇 Additional comments (4)
tests/e2e/common-issues/commonIssues.spec.ts (4)
2-2
: Import added for cleanup helper — good fixThis unblocks the new DB-cleanup flow in tests and resolves the earlier missing import note.
59-59
: Cleanup both created issues — prevents DB bleedGood call deleting both artifacts created in this test. This addresses the data bleed risk between tests.
116-116
: Thorough teardown for search test — looks goodAll created records are removed, aligning with the PR goal to reset to seed between tests.
78-78
: No-op whitespace changeNothing to do here.
Part of #584 point 3 (Reset database back to seed data between tests)
Screenshots of all test passing locally
Summary by CodeRabbit