chore: added teardown script to E2E test suite #1162
Conversation
|
@JohnAllenTech is attempting to deploy a commit to the Codú Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes introduce a new file Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
JohnAllenTech
temporarily deployed
to
production
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
e2e/teardown.ts (3)
1-4: Consider adding type information to importsThe imports look good and are necessary for the script's functionality. However, consider adding type information to improve type safety and developer experience.
You could update the imports as follows:
import dotenv from "dotenv"; import postgres from "postgres";to:
import dotenv from "dotenv"; import postgres from "postgres"; import type { Sql } from "postgres";This change will allow you to properly type the
dbvariable later in the code.
6-10: Approve environment variable check, suggest error handling for DB connectionThe environment variable check is a good practice. However, consider adding error handling for the database connection initialization.
You could update the code as follows:
export const teardown = async () => { try { if (!process.env.DATABASE_URL || !process.env.E2E_USER_ID) throw new Error("Missing env variables for DB clean up script"); let db: Sql; try { db = postgres(process.env.DATABASE_URL as string); } catch (error) { throw new Error(`Failed to connect to the database: ${error.message}`); } // ... rest of the function } catch (err) { console.error("Error while cleaning up DB after E2E test run", err); } };This change adds specific error handling for database connection issues.
21-27: Use console.error for error logging, exports look goodThe error handling and exports look good overall. However, consider using
console.errorfor error logging instead ofconsole.log.Update the error logging as follows:
} catch (err) { console.error("Error while cleaning up DB after E2E test run", err); }This change makes it easier to distinguish between regular logs and error logs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- e2e/teardown.ts (1 hunks)
- playwright.config.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
playwright.config.ts (2)
Line range hint
1-78: Summary: Configuration change looks goodThe addition of the
globalTeardownproperty is the only change in this file, and it correctly implements the requirement for a teardown script as described in the PR objectives. The rest of the configuration remains unchanged, which is appropriate for this specific enhancement.
13-13: LGTM! Verify the existence of the teardown script.The addition of the
globalTeardownproperty is correct and aligns with the PR objective. This will ensure that the teardown script runs after all tests are completed.Let's verify the existence of the teardown script:
✅ Verification successful
Verified: Teardown script exists.
The teardown script
e2e/teardown.tsexists at the specified path, confirming that theglobalTeardownconfiguration is correctly set.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the teardown script # Test: Check if the teardown script exists if fd -p "e2e/teardown.ts$"; then echo "Teardown script found." else echo "Teardown script not found. Please ensure it exists at the specified path." fiLength of output: 97
JohnAllenTech
changed the title
NiallJoeMaher
had a problem deploying
to
production
GitHub Actions
Error
NiallJoeMaher
temporarily deployed
to
production
GitHub Actions
Inactive
✨ Codu Pull Request 💻
Ah... its an issue that we will start to see people complain about
If you run E2E tests over and over again locally you end up with a article page that looks like this
Pull Request details
npx playwright testdoes however run it automatically.You can manually run it by clicking 'Run global teardown'
I have opened an issue for this with playwright here
Any Breaking changes
Associated Screenshots