Add comprehensive golden testing for PDF rendering with pdf-test.pdf and CI workflow#5
Conversation
Co-authored-by: evaisse <28737+evaisse@users.noreply.github.com>
- Added comprehensive golden tests for PDF rendering - Implemented tests for loading, navigation, and UI states - Generated golden master files for visual regression testing - Created test documentation in test/README.md Co-authored-by: evaisse <28737+evaisse@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive golden testing infrastructure for PDF rendering functionality using the test file pdf-test.pdf. The implementation includes unit tests for PDF loading, golden tests for visual verification of the PDF viewer widget in various states, and navigation tests.
Key Changes:
- Added
pdf_rendering_test.dartwith three test groups covering PDF loading, rendering golden tests, and navigation - Created comprehensive test documentation in
test/README.mdexplaining golden tests, test coverage, and CI/CD considerations - Implemented 9 test cases verifying PDF loading, widget rendering in different configurations, and UI states
Reviewed Changes
Copilot reviewed 2 out of 10 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/pdf_rendering_test.dart | New test file implementing golden tests for PDF viewer widget rendering, PDF loading verification, and navigation controls |
| test/README.md | New documentation file providing guidelines for running tests, updating golden files, and describing test coverage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| - Different UI states (loading, no document, etc.) | ||
|
|
||
| ### pdf-test.pdf | ||
| A 10-page test PDF document (39,762 bytes) used for testing PDF loading and rendering. |
There was a problem hiding this comment.
The documentation states the test PDF is a '10-page test PDF document', but the navigation test comments (line 238 in pdf_rendering_test.dart) indicate that pageCount is hardcoded to 1. This inconsistency between documentation and actual behavior should be clarified or corrected.
| A 10-page test PDF document (39,762 bytes) used for testing PDF loading and rendering. | |
| A single-page test PDF document (39,762 bytes) used for testing PDF loading and rendering. |
There was a problem hiding this comment.
Fixed in commit 6d3d8b5. Updated the documentation to clarify that while the PDF file contains 10 pages, the current implementation renders it as a single page since full PDF parsing is not yet implemented.
evaisse
left a comment
evaisse
left a comment
There was a problem hiding this comment.
Can you add a test github workflow ?
|
@copilot can you add a github workflow for CI with steps dart format, flutter analyze --fatal-infos and flutter test ? |
- Created .github/workflows/ci.yml with dart format, flutter analyze, and flutter test steps - Updated test/README.md to clarify PDF page count vs implementation behavior - Applied dart format to all code files for consistency Co-authored-by: evaisse <28737+evaisse@users.noreply.github.com>
Added GitHub CI workflow in commit 6d3d8b5. The workflow includes all requested steps:
The workflow runs on pushes and pull requests to the main/master branches. |
|
@copilot their is errors while executing the CI workflow, can you fix these errors? |
- Removed unnecessary dart:typed_data import from pdf_controller.dart - Removed unused package:pdf/pdf.dart import from pdf_document.dart - Removed unused _document field from PdfDocument class - All CI steps now pass: format check, analyze with --fatal-infos, and tests Co-authored-by: evaisse <28737+evaisse@users.noreply.github.com>
Fixed in commit 3f0e619. The CI errors were caused by analyzer issues in the code:
All CI steps now pass successfully: |
Original prompt
Fixes #4
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.