Add test coverage for Acknowledgement.all() API by MartinP7r · Pull Request #32 · MartinP7r/AckGen

MartinP7r · 2026-01-12T23:14:48Z

Summary

Add test fixtures for plist decoding
Add 4 new tests for Acknowledgement.all() runtime API
Tests cover decoding, case-insensitive sorting, and error handling

Changes

Package.swift - Add test resources
Tests/AckGenTests/Fixtures/ - Test fixture files
Tests/AckGenTests/AcknowledgementAllTests.swift - New test file

Test Results

10/10 tests pass (6 existing + 4 new)

Stacked On

This PR is stacked on #31 (Bug fixes, unit tests, and SwiftArgumentParser migration)

- 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

Copilot

Pull request overview

This PR adds comprehensive test coverage for the Acknowledgement.all() runtime API, building on the unit tests added in PR #31. The PR introduces test fixtures (a valid plist and an invalid UTF-8 file) and 4 new tests covering decoding, sorting, and error handling scenarios.

Changes:

Added test resources configuration to Package.swift
Created test fixtures for plist decoding and invalid data scenarios
Added 4 new tests for the Acknowledgement.all() API covering normal operation, case-insensitive sorting, and error cases

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
Package.swift	Adds `.copy("Fixtures")` to test target resources configuration
Tests/AckGenTests/Fixtures/Acknowledgements.plist	Valid plist fixture with 3 acknowledgements for testing decoding and sorting
Tests/AckGenTests/Fixtures/invalid-utf8-license	Binary file fixture for testing invalid data handling
Tests/AckGenTests/AcknowledgementAllTests.swift	New test file with 4 tests for `Acknowledgement.all()` API

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T13:35:36Z

+
+    func testAllReturnsEmptyArrayForInvalidPlist() {
+        // Given: An invalid file (the invalid-utf8-license fixture)
+        // Note: This test verifies graceful handling of invalid data
+        // The all() method should return empty array for any decode failure
+        let bundle = Bundle.module
+
+        // When: Loading from a plist that would fail to decode
+        // (Using Fixtures subdirectory path)
+        guard let path = bundle.path(forResource: "invalid-utf8-license", ofType: nil, inDirectory: "Fixtures") else {
+            XCTFail("Could not find invalid-utf8-license fixture")
+            return
+        }
+
+        // Verify the file exists and contains invalid data
+        XCTAssertNotNil(FileManager.default.contents(atPath: path))
+
+        // When: Attempting to decode as plist, it should fail gracefully
+        let acks = Acknowledgement.all(fromPlist: "NonExistent", in: bundle)
+
+        // Then: Should return empty array instead of crashing
+        XCTAssertTrue(acks.isEmpty)
+    }


This test has a logic error. It loads the path to the "invalid-utf8-license" fixture (line 74) and verifies it exists (line 80), but then calls Acknowledgement.all(fromPlist: "NonExistent", in: bundle) on line 83. This means the test is not actually testing the invalid UTF-8 file handling - it's just testing a non-existent file again (which is already covered by the previous test).

The test should either:

Call Acknowledgement.all(fromPlist: "invalid-utf8-license", in: bundle) to actually test the invalid UTF-8 file, or

Be removed if the Acknowledgement.all() API doesn't support loading files without the .plist extension, or

Be simplified to just duplicate the missing file test without the misleading comments about invalid UTF-8.

Suggested change

func testAllReturnsEmptyArrayForInvalidPlist() {

// Given: An invalid file (the invalid-utf8-license fixture)

// Note: This test verifies graceful handling of invalid data

// The all() method should return empty array for any decode failure

let bundle = Bundle.module

// When: Loading from a plist that would fail to decode

// (Using Fixtures subdirectory path)

guard let path = bundle.path(forResource: "invalid-utf8-license", ofType: nil, inDirectory: "Fixtures") else {

XCTFail("Could not find invalid-utf8-license fixture")

return

}

// Verify the file exists and contains invalid data

XCTAssertNotNil(FileManager.default.contents(atPath: path))

// When: Attempting to decode as plist, it should fail gracefully

let acks = Acknowledgement.all(fromPlist: "NonExistent", in: bundle)

// Then: Should return empty array instead of crashing

XCTAssertTrue(acks.isEmpty)

}

@copilot instead of removing the test I'd rather test the actual invalid-utf8-license. let's rename it with correct extension and remove guard statement.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T13:39:06Z

+    private func loadAcknowledgementsFromFixture(plistName: String) -> [Acknowledgement] {
+        return Acknowledgement.all(fromPlist: "Fixtures/\(plistName)", in: Bundle.module)
+    }


The helper method loadAcknowledgementsFromFixture on line 15 prefixes the plistName with "Fixtures/" and the Acknowledgement.all() method automatically appends ".plist" extension. However, this creates an inconsistency:

Tests use loadAcknowledgementsFromFixture(plistName: "Acknowledgements") which becomes "Fixtures/Acknowledgements"

The all() method will look for "Fixtures/Acknowledgements.plist"

This works correctly for the valid plist test case, but the approach is fragile. If the helper is intended to be reusable for various fixture types (not just .plist files), the automatic ".plist" extension handling in the helper is misleading.

Consider either:

Documenting that this helper only works with .plist fixtures, or

Making the path construction more explicit in individual tests rather than hiding it in a helper

@copilot open a new pull request to apply changes based on this feedback
Making the path construction more explicit in individual tests rather than hiding it in a helper

Copilot · 2026-02-06T13:40:46Z

@MartinP7r I've opened a new pull request, #34, to work on those changes. Once the pull request is ready, I'll request review from you.

* 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>

Copilot · 2026-02-06T14:07:00Z

@MartinP7r I've opened a new pull request, #35, to work on those changes. Once the pull request is ready, I'll request review from you.

* 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>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

* 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>

* 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>

MartinP7r self-assigned this Feb 6, 2026

MartinP7r marked this pull request as ready for review February 6, 2026 13:31

Copilot AI review requested due to automatic review settings February 6, 2026 13:31

Copilot started reviewing on behalf of MartinP7r February 6, 2026 13:32 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

Update Tests/AckGenTests/AcknowledgementAllTests.swift

ed77756

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings February 6, 2026 13:36

Copilot started reviewing on behalf of MartinP7r February 6, 2026 13:36 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 6, 2026

Test invalid UTF-8 plist handling in Acknowledgement.all() #34

Merged

Copilot AI mentioned this pull request Feb 6, 2026

Remove test helper in favor of explicit path construction #35

Merged

Copilot AI review requested due to automatic review settings February 6, 2026 14:13

Copilot started reviewing on behalf of MartinP7r February 6, 2026 14:14 View session

MartinP7r merged commit d7ef9d3 into feature/bug-fixes-and-tests Feb 6, 2026
4 checks passed

Copilot AI reviewed Feb 6, 2026

View reviewed changes

MartinP7r mentioned this pull request Mar 24, 2026

Improve error feedback and prevent duplicate entries #44

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add test coverage for Acknowledgement.all() API#32

Add test coverage for Acknowledgement.all() API#32
MartinP7r merged 4 commits into
feature/bug-fixes-and-testsfrom
feature/add-cli-tests

MartinP7r commented Jan 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

MartinP7r Feb 6, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

MartinP7r Feb 6, 2026

Uh oh!

Copilot AI commented Feb 6, 2026

Uh oh!

Copilot AI commented Feb 6, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

MartinP7r commented Jan 12, 2026

Summary

Changes

Test Results

Stacked On

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

MartinP7r Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

MartinP7r Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 6, 2026

Uh oh!

Copilot AI commented Feb 6, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants