Skip to content

ci: add unit test workflow#7

Merged
torlando-tech merged 10 commits intomainfrom
ci/add-test-workflow
Mar 27, 2026
Merged

ci: add unit test workflow#7
torlando-tech merged 10 commits intomainfrom
ci/add-test-workflow

Conversation

@torlando-tech
Copy link
Copy Markdown
Owner

Summary

  • Adds GitHub Actions workflow that runs ColumbaAppTests (17 tests) on PRs and pushes to main
  • Clones sibling local packages (reticulum-swift, LXMF-swift, LXST-swift) for the build
  • Runs on macOS 15 with Xcode 16.2, targeting iPhone 16 simulator

Test plan

  • Verify the workflow runs successfully on this PR
  • All 17 tests pass locally

🤖 Generated with Claude Code

Clones sibling local packages (reticulum-swift, LXMF-swift, LXST-swift)
and runs ColumbaAppTests on iOS Simulator.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 27, 2026

Greptile Summary

This PR adds a GitHub Actions CI workflow that builds ColumbaApp and runs its 17-test suite on every PR and push to main. The overall structure is solid — correct triggers, concurrency cancellation, explicit Xcode version selection, and a dedicated dependency-resolution step before testing.\n\nThree minor improvements worth considering before this workflow is relied upon heavily:\n\n- Sibling packages are cloned at HEAD (reticulum-swift, LXMF-swift, LXST-swift) without pinning to a branch, tag, or SHA. A breaking change in any of those repos will fail this workflow even if columba-ios itself hasn't changed, making it harder to attribute failures correctly.\n- The .xcresult bundle is never uploaded. -resultBundlePath TestResults.xcresult is specified but there is no actions/upload-artifact step, so failure diagnostics are lost when the runner is recycled.\n- No SPM caching. With several heavyweight dependencies (MapLibre, GRDB, CryptoSwift), adding an actions/cache step keyed on Package.resolved would meaningfully shorten run times.

Confidence Score: 5/5

Safe to merge — the workflow is functionally correct and all findings are non-blocking P2 suggestions.

No P0 or P1 issues were found. The three observations (unpinned sibling clones, missing artifact upload, no SPM cache) are quality-of-life improvements that do not affect correctness or security.

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/tests.yml Adds a GitHub Actions workflow that runs 17 unit tests on PRs and pushes to main using macOS-15 / Xcode 16.2; sibling local packages are cloned at HEAD, the result bundle is never uploaded, and SPM deps are not cached.

Sequence Diagram

sequenceDiagram
    participant GH as GitHub Actions
    participant Runner as macOS-15 Runner
    participant Sibling as Sibling Repos (GitHub)
    participant SPM as Swift Package Registry
    participant Sim as iPhone 16 Simulator

    GH->>Runner: Trigger (PR / push to main)
    Runner->>Runner: actions/checkout@v4
    Runner->>Sibling: git clone reticulum-swift (HEAD)
    Runner->>Sibling: git clone LXMF-swift (HEAD)
    Runner->>Sibling: git clone LXST-swift (HEAD)
    Runner->>Runner: xcode-select Xcode 16.2
    Runner->>SPM: xcodebuild -resolvePackageDependencies
    SPM-->>Runner: Fetch pinned packages (GRDB, CryptoSwift, MapLibre…)
    Runner->>Sim: xcodebuild test -only-testing:ColumbaAppTests
    Sim-->>Runner: Test results → TestResults.xcresult
    Runner->>Runner: Job ends, .xcresult discarded
Loading

Reviews (1): Last reviewed commit: "ci: add GitHub Actions workflow to run u..." | Re-trigger Greptile

Comment on lines +20 to +23
run: |
git clone https://github.com/torlando-tech/reticulum-swift.git ../reticulum-swift
git clone https://github.com/torlando-tech/LXMF-swift.git ../LXMF-swift
git clone https://github.com/torlando-tech/LXST-swift.git ../LXST-swift
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Sibling packages cloned at HEAD without pinning

The three sibling repos are cloned without specifying a branch, tag, or commit SHA, so every CI run always uses the latest HEAD of each package's default branch. If someone pushes a breaking change to reticulum-swift, LXMF-swift, or LXST-swift, this workflow will start failing even though nothing in columba-ios changed, making it hard to tell whether the failure is local or upstream.

Consider pinning to a specific branch or ref so the build is reproducible and failures are attributable:

git clone --branch main --single-branch https://github.com/torlando-tech/reticulum-swift.git ../reticulum-swift
git clone --branch main --single-branch https://github.com/torlando-tech/LXMF-swift.git ../LXMF-swift
git clone --branch main --single-branch https://github.com/torlando-tech/LXST-swift.git ../LXST-swift

Or, if full reproducibility is needed, add a step that resolves and records the HEAD SHAs of each sibling so failed runs can be replicated exactly.

Comment on lines +31 to +39
- name: Run tests
run: |
xcodebuild test \
-project ColumbaApp.xcodeproj \
-scheme ColumbaApp \
-sdk iphonesimulator \
-destination 'platform=iOS Simulator,name=iPhone 16' \
-only-testing:ColumbaAppTests \
-resultBundlePath TestResults.xcresult
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test result bundle generated but never uploaded

-resultBundlePath TestResults.xcresult writes the result bundle to disk, but there is no actions/upload-artifact step afterward. When a test run fails in CI, the .xcresult bundle is discarded along with the runner and there is no way to inspect the failure from the GitHub Actions UI.

Consider adding an upload step so the bundle is available for debugging:

      - name: Upload test results
        if: always()
        uses: actions/upload-artifact@v4
        with:
          name: TestResults
          path: TestResults.xcresult

Using if: always() ensures the artifact is uploaded even when tests fail, which is when it's most useful.

Comment on lines +28 to +29
- name: Resolve packages
run: xcodebuild -resolvePackageDependencies -project ColumbaApp.xcodeproj -scheme ColumbaApp
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 SPM dependency cache not configured

Every run fetches all remote Swift Package dependencies from scratch (GRDB, CryptoSwift, MapLibre, etc.). Caching the resolved packages significantly reduces run time. A common pattern:

      - name: Cache Swift packages
        uses: actions/cache@v4
        with:
          path: |
            ~/Library/Developer/Xcode/DerivedData
            .build
          key: ${{ runner.os }}-spm-${{ hashFiles('**/Package.resolved') }}
          restore-keys: |
            ${{ runner.os }}-spm-

The Package.resolved hash ensures the cache is invalidated whenever a dependency version changes.

torlando-tech and others added 9 commits March 27, 2026 10:55
The app module failed to build on CI due to missing provisioning
profiles. Skip signing since we only need to run unit tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The runner may not have Xcode 16.2 or iPhone 16 simulator installed.
Dynamically find the latest Xcode 16.x and an available iOS Simulator.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use xcrun simctl to find the first available iPhone simulator by UUID
instead of hardcoding a device name that may not exist on the runner.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use CODE_SIGN_IDENTITY=- (ad-hoc) instead of disabling signing entirely,
so the app and network extension targets can still be built and linked.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Separate the build and test phases so we can see exactly where the
failure occurs. The app module wasn't being compiled before the test
target tried to import it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Build the ColumbaApp target first to ensure the Swift module exists
before the test target tries to import it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use scheme-based build (which resolves SPM deps correctly) as a
separate step before running tests, ensuring the ColumbaApp module
is fully compiled before the test target imports it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@torlando-tech torlando-tech merged commit 8369c24 into main Mar 27, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant