Remove test helper in favor of explicit path construction#35
Merged
Conversation
Co-authored-by: MartinP7r <2669027+MartinP7r@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Update test coverage for Acknowledgement.all() API
Remove test helper in favor of explicit path construction
Feb 6, 2026
MartinP7r
approved these changes
Feb 6, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors test fixture loading to make path construction explicit and transparent, addressing code review feedback from a previous PR. The change removes an abstraction layer that was hiding the fixture path construction logic.
Changes:
- Removed
loadAcknowledgementsFromFixture(plistName:)helper method fromAcknowledgementAllTests - Updated all three test methods to directly call
Acknowledgement.all(fromPlist: "Fixtures/...", in: Bundle.module)with explicit fixture paths
💡 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.
Addresses feedback to make fixture path construction explicit rather than abstracted in a helper method.
Changes
loadAcknowledgementsFromFixture(plistName:)helper methodAcknowledgement.all(fromPlist: "Fixtures/...", in: Bundle.module)with explicit pathsBefore:
After:
This makes the path construction transparent in each test, improving clarity about what fixture is being loaded and how the path is constructed.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.