Conversation
- Add `pkg/graphics/bitmap_test.go` covering `MonochromeBitmap`. - Add `pkg/graphics/engine_test.go` covering `Pipeline`. - Refactor `pkg/graphics/qr.go` to extract helper methods and add TODO/FIXME comments. - Update `.gitignore` to exclude `docs/QR_NITPICKS.md`. - Add `docs/QR_NITPICKS.md` as temporary documentation (ignored).
- Refactor `pkg/connection` to use `PrinterService` interface, abstracting Windows API calls. - Unify `WindowsPrintConnector` struct to be platform-agnostic. - Add `pkg/connection/windows_connector_test.go` with comprehensive unit tests using mocks. - Add `docs/CONN_NITPICKS.md` documenting technical debt and issues. - Add `docs/CONN_NITPICKS.md` to `.gitignore` allow-list. - Add TODO/FIXME comments referencing nitpicks.
- add Open, Close, StartDoc, EndDoc, AbortDoc, and Write methods to RealPrinterService - improve error handling and logging during print job operations Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
- Add `escpos_profile_test.go` and `escpos_encoding_test.go` - Add `docs/PROFILE_NITPICKS.md` with observations - Add TODO/FIXME comments in `pkg/profile` based on nitpicks - Allow `docs/PROFILE_NITPICKS.md` in .gitignore
- update build command to specify output executable Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
Contributor
|
👋 Thanks for opening this PR, @adcondev! Here's what will happen next:
Please make sure:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces comprehensive unit tests for the profile, connection, and graphics packages while refactoring the Windows printer connector architecture for better testability. The changes improve code maintainability through interface-based abstraction and add documentation for technical debt.
Key changes:
- Introduces
PrinterServiceinterface to abstract OS-specific printer operations, enabling dependency injection and mocking for unit tests - Adds 100+ unit tests covering profile creation, encoding, graphics processing, bitmap operations, and Windows connector functionality using the testify framework
- Refactors QR code generation into smaller, more focused helper methods for improved maintainability
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/connection/service.go |
Defines new PrinterService interface for OS-agnostic printer operations |
pkg/connection/windows_connector.go |
Refactored connector using injected PrinterService for testability |
pkg/connection/windows.go |
Implements RealPrinterService with Windows syscalls, separated from connector logic |
pkg/connection/windows_stub.go |
Implements StubPrinterService for non-Windows platforms |
pkg/connection/windows_connector_test.go |
Comprehensive unit tests with mocked printer service |
pkg/profile/escpos_profile_test.go |
Unit tests validating profile factory functions |
pkg/profile/escpos_encoding_test.go |
Unit tests for character encoding and fallback behavior |
pkg/graphics/engine_test.go |
Unit tests for image pipeline processing, resizing, and dithering |
pkg/graphics/bitmap_test.go |
Unit tests for monochrome bitmap operations |
pkg/graphics/qr.go |
Refactored QR generation into smaller methods with added TODO/FIXME comments |
pkg/profile/escpos_encoding.go |
Added FIXME/TODO comments documenting technical debt |
pkg/profile/escpos_profile.go |
Added TODO comment about hardcoded model factories |
pkg/document/executor.go |
Modified DotsPerLine calculation to subtract 8mm from paper width |
docs/CONN_NITPICKS.md |
Documents architectural issues and refactoring needs for connection package |
docs/PROFILE_NITPICKS.md |
Documents nitpicks and observations for profile package |
go.mod |
Adds testify v1.7.0 dependency for unit testing |
go.sum |
Updates dependency checksums |
Taskfile.yml |
Updates build command to output specific binary name |
examples/tickets/simple_ticket.json |
Adds new example ticket document |
examples/document/qrcode/qr_scenario_wifi.json |
Updates WiFi scenario text and separator |
.gitignore |
Adds nitpick documentation files to tracking |
Comments suppressed due to low confidence (1)
pkg/connection/windows_connector.go:94
- This call to 'recover' has no effect because the enclosing function is never called using a defer statement.
} else if r := recover(); r != nil {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
- Removed error handling from calculateModuleSize method as it no longer returns an error - Updated method signature to reflect the change Signed-off-by: Adrián Constante <ad_con.reload@proton.me>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request introduces several architectural improvements to the Windows printer connector, adds documentation for nitpicks and technical debt, updates example documents, and refines the build process. The most important changes are the refactoring of the Windows connector to use a new
PrinterServiceinterface for better testability and maintainability, along with the addition of nitpick documentation to guide future improvements.Connector Architecture Refactor
PrinterServiceinterface inpkg/connection/service.goto abstract OS-specific printer operations, enabling easier mocking and unit testing. ([pkg/connection/service.goR1-R12](https://github.com/adcondev/poster/pull/74/files#diff-1de8c22e5bbb4b5e793b0646651488b76b5169b87f69b4f50825ea5e070f910aR1-R12))RealPrinterServicestruct inpkg/connection/windows.go, and updatedWindowsPrintConnectorinpkg/connection/windows_connector.goto use this service for all printer operations. This decouples the connector logic from direct syscalls and global state. ([[1]](https://github.com/adcondev/poster/pull/74/files#diff-b6589a8b1435726955877456cf1c8cb290650db15992ec91f09c4f6d8c34730bL33-R106),[[2]](https://github.com/adcondev/poster/pull/74/files#diff-83ab87b3f16e61950442eac01a5cd95253736a617ed8efdfe4c14f088f576cceR1-R122))pkg/connection/windows.goand moved it topkg/connection/windows_connector.go, clarifying separation of concerns and improving testability. ([[1]](https://github.com/adcondev/poster/pull/74/files#diff-b6589a8b1435726955877456cf1c8cb290650db15992ec91f09c4f6d8c34730bL33-R106),[[2]](https://github.com/adcondev/poster/pull/74/files#diff-b6589a8b1435726955877456cf1c8cb290650db15992ec91f09c4f6d8c34730bL206-L210))Documentation and Nitpicks
docs/CONN_NITPICKS.mdanddocs/PROFILE_NITPICKS.mdto document architectural issues, implementation gaps, and refactoring needs for the connection and profile packages, providing a roadmap for future improvements. ([[1]](https://github.com/adcondev/poster/pull/74/files#diff-48ad39b3b8cb4135a14b3e023337c36e1fd1457881acbe03649ad851921ffa42R1-R18),[[2]](https://github.com/adcondev/poster/pull/74/files#diff-187c1ac8bf9bce4bf00f26efbdaf6ab603c59d25588b187c499e347d90ecc6d0R1-R14))Build and Dependency Updates
Taskfile.ymlto output a specific binary (poster.exe) and added new dependencies (includingtestifyfor testing) togo.mod. ([[1]](https://github.com/adcondev/poster/pull/74/files#diff-cd2d359855d0301ce190f1ec3b4c572ea690c83747f6df61c9340720e3d2425eL86-R86),[[2]](https://github.com/adcondev/poster/pull/74/files#diff-33ef32bf6c23acb95f5902d7097b7a1d5128ca061167ec0716715b0b9eeaa5f6R6-R21))Example and Test Document Updates
examples/tickets/simple_ticket.json) and updated existing QR code scenario documents for improved clarity and demonstration. ([[1]](https://github.com/adcondev/poster/pull/74/files#diff-a5ea62aeda921f81a4a1866bc1ffaf29449ebecfc4c33a8774eb9b52c303a823R1-R61),[[2]](https://github.com/adcondev/poster/pull/74/files#diff-0b870cbda057ac79386d19cfb7f1556810401f335e5be52ff6748aec98c178b1L13-R13),[[3]](https://github.com/adcondev/poster/pull/74/files#diff-0b870cbda057ac79386d19cfb7f1556810401f335e5be52ff6748aec98c178b1L25-R25))Type of Change
Component(s) Affected
composer- ESC/POS protocol composerconnection- Printer connectionscommands- ESC/POS command implementationsdocument- Document processinggraphics- Image processingprinter- Barcode generationprofile- Printer profilesservice- High-level printer servicegithub- GitHub related files and workflowsHow Has This Been Tested?
Test Configuration
Checklist
Screenshots/Examples
Breaking Changes
Additional Notes