Test invalid UTF-8 plist handling in Acknowledgement.all()#34
Merged
Conversation
…est it Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor test for Acknowledgement.all() API with invalid UTF-8
Test invalid UTF-8 plist handling in Acknowledgement.all()
Feb 6, 2026
MartinP7r
approved these changes
Feb 6, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a test that was inadvertently testing the wrong scenario. The test testAllReturnsEmptyArrayForInvalidPlist was supposed to verify that Acknowledgement.all() gracefully handles invalid UTF-8 content by returning an empty array, but it was actually testing a non-existent file (duplicating the missing file test). The fix ensures the test correctly validates the invalid UTF-8 handling behavior.
Changes:
- Renamed the fixture file from
invalid-utf8-licensetoinvalid-utf8-license.plistto satisfy the.plistextension requirement ofBundle.path(forResource:ofType:) - Updated the test to use the
loadAcknowledgementsFromFixture()helper method with the correct fixture name instead of testing a non-existent file - Simplified test code by removing manual path verification and guard statements, now consistently using the helper method
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Tests/AckGenTests/Fixtures/invalid-utf8-license.plist | Added fixture file containing invalid UTF-8 bytes to test graceful error handling |
| Tests/AckGenTests/AcknowledgementAllTests.swift | Fixed test to actually load and verify the invalid UTF-8 fixture instead of testing a non-existent file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MartinP7r
added a commit
that referenced
this pull request
Feb 6, 2026
* Add tests for Acknowledgement.all() with fixture plist - Create test fixtures directory with sample plist and invalid UTF-8 file - Add AcknowledgementAllTests with 4 tests covering: - Decoding from fixture plist - Case-insensitive sorting behavior - Graceful handling of missing plist (returns empty array) - Graceful handling of invalid data (returns empty array) - Update Package.swift to include test resources * Update Tests/AckGenTests/AcknowledgementAllTests.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Test invalid UTF-8 plist handling in Acknowledgement.all() (#34) * Initial plan * Rename invalid-utf8-license to .plist and refactor test to actually test it Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> * Remove test helper in favor of explicit path construction (#35) * Initial plan * Remove helper method and make path construction explicit in tests Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com>
MartinP7r
added a commit
that referenced
this pull request
Feb 7, 2026
* Add tests for Acknowledgement.all() with fixture plist - Create test fixtures directory with sample plist and invalid UTF-8 file - Add AcknowledgementAllTests with 4 tests covering: - Decoding from fixture plist - Case-insensitive sorting behavior - Graceful handling of missing plist (returns empty array) - Graceful handling of invalid data (returns empty array) - Update Package.swift to include test resources * Update Tests/AckGenTests/AcknowledgementAllTests.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Test invalid UTF-8 plist handling in Acknowledgement.all() (#34) * Initial plan * Rename invalid-utf8-license to .plist and refactor test to actually test it Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> * Remove test helper in favor of explicit path construction (#35) * Initial plan * Remove helper method and make path construction explicit in tests Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com>
MartinP7r
added a commit
that referenced
this pull request
Mar 8, 2026
* empty commit * Add CLAUDE.md and improvement plan documentation * Add unit tests for AckGenCore - Replace broken integration test with 6 comprehensive unit tests - Test initialization, Comparable, encoding/decoding, and array operations - All tests passing This fixes the issue where 'swift test' was running 0 tests * Migrate CLI to SwiftArgumentParser and fix critical bugs Breaking changes: - Migrate from positional arguments to named flags Old: ackgen path/to/file.plist 1 "Title" New: ackgen --output path/to/file.plist --settings --title "Title" - Add --help and --version commands (auto-generated) Bug fixes: - Fix force unwrap crash when LICENSE files contain invalid UTF-8 - Add proper exit codes (Darwin.exit) so Xcode build phases fail on errors - Replace error returns with ValidationError throws for better error messages Dependencies: - Add swift-argument-parser 1.3.0 Updated Example scripts to use new CLI syntax. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * Add validation for empty acknowledgements Throws ValidationError when no license files are found, preventing silent generation of empty plist files. Includes helpful message directing users to resolve SPM packages. * Add test coverage for Acknowledgement.all() API (#32) * Add tests for Acknowledgement.all() with fixture plist - Create test fixtures directory with sample plist and invalid UTF-8 file - Add AcknowledgementAllTests with 4 tests covering: - Decoding from fixture plist - Case-insensitive sorting behavior - Graceful handling of missing plist (returns empty array) - Graceful handling of invalid data (returns empty array) - Update Package.swift to include test resources * Update Tests/AckGenTests/AcknowledgementAllTests.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Test invalid UTF-8 plist handling in Acknowledgement.all() (#34) * Initial plan * Rename invalid-utf8-license to .plist and refactor test to actually test it Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> * Remove test helper in favor of explicit path construction (#35) * Initial plan * Remove helper method and make path construction explicit in tests Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com> * Update Tests/AckGenTests/AcknowledgementTests.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update CLAUDE.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Tests/AckGenTests/AcknowledgementAllTests.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Sources/AckGenCLI/AckGen.swift Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * docs: update README and CLAUDE.md to reflect ArgumentParser migration - Update CLI usage examples from positional args to --output/--settings/--title flags - Fix CLAUDE.md claiming "no external dependencies" (now has swift-argument-parser) - Fix --settings flag syntax in CLAUDE.md (it's a flag, not an option) - Mark "Add tests" as done in README TODO list - Document Settings.bundle generation in README --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Happy <yesreply@happy.engineering> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com>
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.
The test
testAllReturnsEmptyArrayForInvalidPlistwas verifying a non-existent file instead of theinvalid-utf8-licensefixture it claimed to test, effectively duplicating the missing file test.Changes:
invalid-utf8-license→invalid-utf8-license.plist(required byBundle.path(forResource:ofType: "plist"))loadAcknowledgementsFromFixture(plistName: "invalid-utf8-license")instead of"NonExistent"The test now correctly verifies that
Acknowledgement.all()returns an empty array when attempting to decode invalid UTF-8 content.💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.