🧪 [testing improvement] Add E2E test for AppController handlePrint error#319
🧪 [testing improvement] Add E2E test for AppController handlePrint error#319guitarbeat wants to merge 2 commits into
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
User does not have a PR Review subscription. Go to Team management and add this email to the PR Review subscription. |
Reviewer's GuideAdds a new Playwright E2E test that verifies AppController.handlePrint shows a detailed error toast when ExportService.handlePrint rejects, and documents related testing learnings in the Jules testing notes. Sequence diagram for E2E test of AppController.handlePrint error handlingsequenceDiagram
actor E2ETest
participant AppController
participant ExportService
E2ETest->>AppController: handlePrint
AppController->>AppController: getFilledPageCount
AppController->>ExportService: handlePrint
alt [ExportService rejects and error toast shown with error details]
AppController->>AppController: handlePrint
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
| await expect(page.locator('#upload-status')).toContainText('Imported 1 of 1 pages', { timeout: 10000 }); | ||
|
|
||
| // 2. Mock exportService.handlePrint to reject | ||
| await page.evaluate(() => { |
There was a problem hiding this comment.
Great job covering the error path for print handling. The test is clear and well-structured. However, consider using Playwright's page.route or page.exposeFunction to mock backend/service behavior instead of mutating window.app directly. This helps decouple the test from the internal structure and reduces brittleness if the app refactors its global namespace.
| }); | ||
|
|
||
| // 3. Trigger print | ||
| const printBtn = page.locator('#printBtn'); |
There was a problem hiding this comment.
The selector #printBtn is used directly. If this is a stable, documented test id, that's fine. Otherwise, consider using a data-testid attribute for better resilience to UI changes.
| await printBtn.click(); | ||
|
|
||
| // 4. Check for toast error | ||
| const toast = page.locator('.toast.toast-error'); |
There was a problem hiding this comment.
Consider adding a comment or assertion to ensure that no duplicate error toasts are present, especially if the print button can be clicked multiple times. This will help catch issues with repeated error handling.
| import { test, expect } from '@playwright/test'; | ||
| import { jsPDF } from 'jspdf'; | ||
|
|
||
| function createPdfBuffer(label) { |
There was a problem hiding this comment.
The helper function createPdfBuffer is a nice touch for test setup. If you have other tests that need PDF generation, consider moving this utility to a shared test utils file for reuse.
🎯 What: Added an E2E test to cover the error handling logic in
AppController.handlePrintwhen theExportServicerejects the print request.📊 Coverage: Ensures that if printing fails, the user is notified with an error toast message containing the specific error details.
✨ Result: Improved test coverage by validating the
catchblock inAppController.handlePrint.PR created automatically by Jules for task 15629631318699926568 started by @guitarbeat
Summary by Sourcery
Add an end-to-end test covering print error handling and document the related testing learnings.
Documentation:
Tests: