Skip to content

Expand test coverage and improve stability - #4

Merged
maheshrijal merged 1 commit into
mainfrom
feat/add-invoice-tests
Jan 2, 2026
Merged

maheshrijal merged 1 commit into
mainfrom
feat/add-invoice-tests

Conversation

@maheshrijal

Copy link
Copy Markdown
Owner

This PR adds comprehensive E2E tests for the Rebill invoice generator, including item management, form persistence, tax/discount calculations, and multi-page PDF generation. It also fixes a stability issue in the deepMerge function and updates the Makefile with test targets.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying rebill with  Cloudflare Pages  Cloudflare Pages

Latest commit: fc47f2f
Status: ✅  Deploy successful!
Preview URL: https://96858fd9.rebill.pages.dev
Branch Preview URL: https://feat-add-invoice-tests.rebill.pages.dev

View logs

@greptile-apps

greptile-apps Bot commented Jan 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR significantly expands test coverage for the Rebill invoice generator and fixes critical stability issues. The changes include:

Test Coverage Improvements:

  • Added comprehensive E2E tests for history management (loading/deleting invoices)
  • Added JSON import/export tests covering textarea import, file download, and file upload flows
  • Added tests for item management (removal, total calculations)
  • Added tests for invoice number increment/decrement functionality
  • Added form persistence test to verify auto-save behavior
  • Added multi-page PDF generation test with 30 items
  • Fixed pdf-parse import by using direct path to lib/pdf-parse.js to avoid broken debug mode in package

Stability Fixes:

  • Fixed race condition in main.js by moving isApplyingData = true before calling applyDataToForm() and importJsonData() (src/js/main.js:64, 151). This prevents the form input listener from triggering unwanted syncFromForm() calls during programmatic data application
  • Simplified deepMerge() in storage.js by removing redundant hasUnsafeKeys() recursive validation. Protection against prototype pollution is maintained through top-level key filtering in the deepMerge function itself
  • Removed unnecessary meta.showInvoice manipulation in json-io.js

Build Infrastructure:

  • Added test and test-report targets to Makefile for convenient test execution

All changes are well-tested and improve application reliability.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The PR includes comprehensive test coverage for all new and modified functionality, fixes legitimate bugs with clear reasoning, and simplifies code while maintaining security. The race condition fix addresses a real timing issue, and the deepMerge simplification removes redundant checks without compromising prototype pollution protection. All test files include proper assertions and edge case coverage.
  • No files require special attention

Important Files Changed

Filename Overview
src/js/modules/storage.js Simplified deepMerge by removing redundant hasUnsafeKeys function while maintaining prototype pollution protection
src/js/main.js Fixed race condition by setting isApplyingData flag before calling functions that modify form
tests/advanced-features.spec.js Added comprehensive tests for history, JSON I/O, and multi-page PDF generation; fixed pdf-parse import
tests/rebill.spec.js Added tests for item removal, invoice number increment/decrement, and form persistence; fixed pdf-parse import

Sequence Diagram

sequenceDiagram
    participant User
    participant Test
    participant Browser
    participant App
    participant LocalStorage
    participant PDF

    Note over Test,Browser: Test Setup
    Test->>Browser: goto('/')
    Test->>Browser: localStorage.clear()
    Test->>Browser: reload()

    Note over Test,PDF: Form Filling & Invoice Generation
    Test->>Browser: fill('#sellerName', 'Test Seller')
    Test->>Browser: fill('#invoiceNumber', 'INV-001')
    Test->>Browser: fill('.item-description', 'Test Item')
    Browser->>App: input event
    App->>LocalStorage: saveDraft(data)
    
    Test->>Browser: click('Generate Bill')
    Browser->>App: generateBill()
    App->>App: syncFromForm()
    App->>LocalStorage: saveToHistory(data)
    App->>App: renderInvoice()

    Note over Test,PDF: PDF Download & Validation
    Test->>Browser: click('#downloadBtn')
    Browser->>PDF: generate PDF with html2pdf
    PDF-->>Test: download event
    Test->>Test: parse PDF with pdf-parse
    Test->>Test: verify content

    Note over Test,LocalStorage: History & JSON I/O Tests
    Test->>Browser: click('#historyToggle')
    Browser->>App: toggleHistoryPanel()
    App->>LocalStorage: getHistory()
    LocalStorage-->>App: history entries
    
    Test->>Browser: click('.history-item')
    Browser->>App: loadFromHistory(id)
    App->>App: isApplyingData = true
    App->>App: applyDataToForm(data)
    App->>App: syncFromForm()
    App->>App: isApplyingData = false

    Note over Test,App: JSON Import Test
    Test->>Browser: fill('#jsonData', JSON.stringify(data))
    Test->>Browser: click('#importJsonBtn')
    Browser->>App: importJsonData()
    App->>App: isApplyingData = true
    App->>App: deepMerge(base, parsed)
    App->>App: applyDataToForm(merged)
    App->>App: isApplyingData = false
Loading

@github-actions

github-actions Bot commented Jan 2, 2026

Copy link
Copy Markdown

Playwright test results

passed  72 passed

Details

stats  72 tests across 2 suites
duration  1 minute, 43 seconds
commit  fc47f2f

@maheshrijal
maheshrijal merged commit 9282794 into main Jan 2, 2026
3 checks passed
@maheshrijal
maheshrijal deleted the feat/add-invoice-tests branch January 2, 2026 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant