feature: add solution on daily 21 KT #268
Conversation
WalkthroughThis pull request introduces a new Playwright test suite for a ticket purchasing feature. The test script focuses on simulating a ticket purchase process on a concert ticket website. It includes a Changes
Sequence DiagramsequenceDiagram
participant User
participant Website
participant LoginPage
participant TicketSection
participant Cart
User->>LoginPage: Navigate and login
LoginPage-->>Website: Authenticate user
User->>TicketSection: Select ticket
User->>TicketSection: Set ticket quantity
User->>TicketSection: Confirm purchase
TicketSection-->>Cart: Add ticket to cart
User->>Cart: Verify cart details
User->>Website: Navigate back to home page
Possibly related PRs
Suggested reviewers
Poem
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 (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
daily-challenges/2024-10/21/khanh5923_21.spec.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
daily-challenges/2024-10/21/khanh5923_21.spec.ts
[error] 4-4: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 28-28: This let declares a variable that is only assigned once.
'miniCart' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 31-31: This let declares a variable that is only assigned once.
'priceProductCart' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 33-33: This let declares a variable that is only assigned once.
'priceProductSection' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
| @@ -0,0 +1,36 @@ | |||
| import { test, expect } from '@playwright/test'; | |||
| test.describe('khanh5923_20-Buy ticket', () => { | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor naming mismatch in the test suite.
The describe block is titled 'khanh5923_20-Buy ticket', but the test name is 'khanh5923_21'. To maintain consistency, consider adjusting one of them.
| await dialog.accept(); | ||
| }); | ||
| await page.click(`//button[@class='btn-confirm']`); | ||
| let miniCart = page.locator(`//div[@id="cart-icon"]//span[@id="cart-count"]`); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use const instead of let for variables that are never reassigned.
The variables miniCart, priceProductCart, and priceProductSection remain unchanged after their initial assignments, so const is more appropriate.
-let miniCart = page.locator(`//div[@id="cart-icon"]//span[@id="cart-count"]`);
+const miniCart = page.locator(`//div[@id="cart-icon"]//span[@id="cart-count"]`);
-let priceProductCart = (await page.locator(`//tbody[@id="cart-items"]//td[2]`).textContent()) || '';
+const priceProductCart = (await page.locator(`//tbody[@id="cart-items"]//td[2]`).textContent()) || '';
-let priceProductSection = (await page.locator(`${locatorTicket}::ul//span[@class="price"]`).textContent()) || '';
+const priceProductSection = (await page.locator(`${locatorTicket}::ul//span[@class="price"]`).textContent()) || '';Also applies to: 31-31, 33-33
🧰 Tools
🪛 Biome (1.9.4)
[error] 28-28: This let declares a variable that is only assigned once.
'miniCart' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
| test('khanh5923_21', async ({ page }) => { | ||
| const locatorTicket = `//div[@id='ticket-section']//h3[text()='${dataTest.ticketName}']/following-sibling`; | ||
| await page.click(`${locatorTicket}::button`); | ||
| await page.fill(`//input[@id='ticket-quantity']`, `1`); | ||
| page.on('dialog', async dialog => { | ||
| await dialog.accept(); | ||
| }); | ||
| await page.click(`//button[@class='btn-confirm']`); | ||
| let miniCart = page.locator(`//div[@id="cart-icon"]//span[@id="cart-count"]`); | ||
| expect(await miniCart.textContent()).toBe(`${dataTest.ticketQuantity}`); | ||
| await miniCart.click(); | ||
| let priceProductCart = (await page.locator(`//tbody[@id="cart-items"]//td[2]`).textContent()) || ''; | ||
| await page.click(`//button[@id="back-home-btn"]`); | ||
| let priceProductSection = (await page.locator(`${locatorTicket}::ul//span[@class="price"]`).textContent()) || ''; | ||
| expect(convertPrice(priceProductCart)).toBe(convertPrice(priceProductSection)); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding additional negative and edge-case coverage.
No negative flows are tested, such as a failed login, invalid ticket quantity, or unavailability of the ticket. Strengthening test coverage would improve confidence in the feature.
🧰 Tools
🪛 Biome (1.9.4)
[error] 28-28: This let declares a variable that is only assigned once.
'miniCart' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 31-31: This let declares a variable that is only assigned once.
'priceProductCart' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 33-33: This let declares a variable that is only assigned once.
'priceProductSection' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
| function convertPrice(price: string) { | ||
| return parseInt(price.replace(/\D/g, '')); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefer Number.parseInt over the global parseInt.
As recommended by static analysis, using Number.parseInt enhances consistency and clarity.
-function convertPrice(price: string) {
- return parseInt(price.replace(/\D/g, ''));
+function convertPrice(price: string) {
+ return Number.parseInt(price.replace(/\D/g, ''), 10);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function convertPrice(price: string) { | |
| return parseInt(price.replace(/\D/g, '')); | |
| function convertPrice(price: string) { | |
| return Number.parseInt(price.replace(/\D/g, ''), 10); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
| test('khanh5923_21', async ({ page }) => { | ||
| const locatorTicket = `//div[@id='ticket-section']//h3[text()='${dataTest.ticketName}']/following-sibling`; | ||
| await page.click(`${locatorTicket}::button`); | ||
| await page.fill(`//input[@id='ticket-quantity']`, `1`); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use the dataTest.ticketQuantity for consistency.
Currently, the code uses 1 directly instead of referencing dataTest.ticketQuantity. This can cause inconsistencies if the quantity requirement changes.
-await page.fill(`//input[@id='ticket-quantity']`, `1`);
+await page.fill(`//input[@id='ticket-quantity']`, `${dataTest.ticketQuantity}`);Committable suggestion skipped: line range outside the PR's diff.
Dạ mọi người xem giúp em bài này nhé. Em cảm ơn.
Summary by CodeRabbit
New Features
Tests