-
-
Notifications
You must be signed in to change notification settings - Fork 386
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
✅ Add test for guest to logged in user migration #1544
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis pull request modifies several test cases by explicitly specifying poll names when calling the poll creation method. The changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as GuestPollTest
participant NP as NewPollPage
participant LP as LoginPage
Test->>NP: createPollAndCloseDialog({ name: "Board Meeting" })
NP-->>Test: Poll created with "Board Meeting"
Test->>LP: Navigate to login page (LP.goto())
LP->>LP: Wait for "Welcome" text
Test->>LP: login({ email: "[email protected]" })
LP-->>Test: Verification code processing
Test->>NP: Assert poll title remains correct
sequenceDiagram
participant Tester as Test
participant LP as LoginPage
Tester->>LP: goto() → Navigate to "/login"
LP->>LP: Wait for "Welcome" indicator
Tester->>LP: login({ email: "[email protected]" })
LP->>LP: Retrieve and fill verification code
LP-->>Tester: "Finish Logging In" confirmation
Possibly related PRs
Poem
✨ Finishing Touches
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 (
|
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
🧹 Nitpick comments (3)
apps/web/tests/login-page.ts (1)
13-25
: Consider enhancing error handling and documentation.While the login flow is well-structured, consider these improvements:
- Add error handling for failed verification code retrieval
- Add timeout handling for verification code wait
- Add JSDoc comments to document the method's behavior and parameters
+/** + * Logs in a user using email verification. + * @param {Object} params - The login parameters + * @param {string} params.email - The email address to log in with + * @throws {Error} When verification code retrieval fails + */ async login({ email }: { email: string }) { // Fill in registration form await this.page.getByPlaceholder("[email protected]").fill(email); await this.page .getByRole("button", { name: "Continue with Email", exact: true }) .click(); // Handle verification code - const code = await getCode(email); + const code = await getCode(email).catch((error) => { + throw new Error(`Failed to retrieve verification code: ${error.message}`); + }); await this.page.getByText("Finish Logging In").waitFor({ + timeout: 10000 + }); await this.page.getByPlaceholder("Enter your 6-digit code").fill(code); }apps/web/tests/new-poll-page.ts (1)
17-40
: Consider enhancing error handling and maintainability.While the implementation is functional, consider these improvements:
- Add error handling for failed page interactions
- Extract hardcoded strings to constants
- Add JSDoc documentation
+const POLL_DEFAULTS = { + LOCATION: "Online", + DESCRIPTION: "Hey everyone, what time can you meet?", + DAYS: [5, 7, 10, 15] +} as const; + +/** + * Creates a new poll with the specified name. + * @param {Object} params - The poll parameters + * @param {string} params.name - The name of the poll + * @returns {Promise<PollPage>} The created poll page + * @throws {Error} When page interactions fail + */ async createPoll({ name }: { name: string }) { const page = this.page; await page.fill('[placeholder="Monthly Meetup"]', name); // click on label to focus on input await page.click('text="Location"'); - await page.keyboard.type("Online"); + await page.keyboard.type(POLL_DEFAULTS.LOCATION); await page.click('text="Description"'); - await page.keyboard.type("Hey everyone, what time can you meet?"); + await page.keyboard.type(POLL_DEFAULTS.DESCRIPTION); await page.click('[title="Next month"]'); // Select a few days - await page.click("text=/^5$/"); - await page.click("text=/^7$/"); - await page.click("text=/^10$/"); - await page.click("text=/^15$/"); + for (const day of POLL_DEFAULTS.DAYS) { + await page.click(`text=/^${day}$/`).catch((error) => { + throw new Error(`Failed to select day ${day}: ${error.message}`); + }); + } await page.click('text="Create poll"'); return new PollPage(page); }apps/web/tests/guest-to-user-migration.spec.ts (1)
51-71
: Fix typo in comment and LGTM!The new test case for guest to logged-in user migration is well-structured and comprehensive. However, there's a typo in the comment on line 63: "lplogin" should be "login".
- // Step 3: Complete lplogin + // Step 3: Complete login
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/tests/create-delete-poll.spec.ts
(1 hunks)apps/web/tests/edit-options.spec.ts
(1 hunks)apps/web/tests/guest-to-user-migration.spec.ts
(3 hunks)apps/web/tests/login-page.ts
(1 hunks)apps/web/tests/new-poll-page.ts
(1 hunks)apps/web/tests/register-page.ts
(0 hunks)apps/web/tests/vote-and-comment.spec.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/tests/register-page.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Unit tests
- GitHub Check: Type check
- GitHub Check: Integration tests
- GitHub Check: Linting
🔇 Additional comments (8)
apps/web/tests/login-page.ts (2)
5-6
: LGTM! Well-structured constructor.The constructor follows TypeScript best practices with proper encapsulation and dependency injection.
8-11
: LGTM! Robust page navigation.The method follows Playwright best practices by ensuring the page is fully loaded before proceeding.
apps/web/tests/create-delete-poll.spec.ts (1)
18-18
: LGTM! Consistent with method signature update.The change correctly aligns with the updated
createPollAndCloseDialog
method signature and is validated by the subsequent title assertion.apps/web/tests/edit-options.spec.ts (1)
14-16
: LGTM! Clean and consistent implementation.The change correctly implements the updated method signature using proper TypeScript object destructuring.
apps/web/tests/new-poll-page.ts (1)
11-16
: LGTM! Well-structured method with clear responsibility.The method follows good practices with proper type annotations and single responsibility.
apps/web/tests/guest-to-user-migration.spec.ts (2)
5-5
: LGTM!The import of
LoginPage
class is correctly added to support the new test case.
30-30
: LGTM!The update to
createPollAndCloseDialog
call with explicit poll name improves test clarity and maintainability.apps/web/tests/vote-and-comment.spec.ts (1)
24-26
: LGTM!The update to
createPollAndCloseDialog
call with explicit poll name improves test clarity and maintains consistency with the email subject test on line 58.
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
🧹 Nitpick comments (1)
apps/web/tests/authentication.spec.ts (1)
43-43
: Add await and navigation verification.Good addition of explicit navigation, but it needs two improvements:
- Add
await
to prevent potential race conditions- Add navigation verification to ensure the page loaded successfully
Apply this diff:
- registerPage.goto(); + await registerPage.goto(); + await expect(page.getByText("Create Your Account")).toBeVisible();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/tests/authentication.spec.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Unit tests
- GitHub Check: Integration tests
- GitHub Check: Type check
- GitHub Check: Linting
Summary by CodeRabbit
New Features
LoginPage
class has been introduced to automate the login process.Improvements