Skip to content

Test coverage for pfw tool#5

Merged
mbrandonw merged 39 commits intomainfrom
tests
Jan 28, 2026
Merged

Test coverage for pfw tool#5
mbrandonw merged 39 commits intomainfrom
tests

Conversation

@mbrandonw
Copy link
Copy Markdown
Member

The alpha release of this tool had no tests whatsoever and we just smashed everything together to make it work. But now that we are gearing up to go live, and it's more likely that people will want to PR fixes, we should have some test coverage.

I added dependencies for controlling the local auth server, the file system, the "open in browser" functionality, the Point-Free server, and finally the "whoami" hook. With that done we can mock out a variety of scenarios to make sure everything works.

There are some known bugs in the tool now (for claude and when using --path), but we will fix those in separate PRs with appropriate tests to show the fixes.

Comment on lines +10 to +13
protocol Auth: Sendable {
func start() async throws -> URL
func waitForToken() async throws -> String
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This represents the local auth server we start up and ask for a token. There's an in-memory version below that can just immediately give back the callback URL and token when asked. We should beef it up to allow emitting errors too and get test coverage on that, but we can do that later.

import Foundation
import ZIPFoundation

protocol FileSystem: Sendable {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is an abstraction of a file system held in memory. We've used this kind of dependency quite a bit in the past, but for this I needed to beef it up quite a bit since pfw does a lot more with the file system (unzips, creates directories, etc.).

import Foundation
import XCTestDynamicOverlay

protocol OpenInBrowser: Sendable {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just a simple interface to opening the browser to a specific URL.

Comment on lines +8 to +10
protocol PointFreeServer: Sendable {
func downloadSkills(token: String, machine: UUID, whoami: String) async throws -> Data
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

An interface to abstract away downloading skills from our Point-Free server.

Comment thread Sources/pfw/Install.swift
Comment on lines +32 to +37
func validate() throws {
guard (tool != nil) != (path != nil) else {
throw ValidationError("Provide either --tool or --path.")
}
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We now require either tool or path be provided.

@testable import pfw

nonisolated(nonsending)
func assertCommand(
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added an inline snapshot testing tool for testing CLI commands. It's rough right now, but I think there are some interesting directions we can take it.

}

// TODO: Explore a dependency alternative to this.
func withCapturedStdout(_ body: () async throws -> Void) async rethrows -> String {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This captures stdout, which is cool, but also means we can't run tests in parallel because we will get interleaving logs from other tests. An alternative would be to put printing behind a dependency but then that means you have to know to use the dependency instead of print.

Comment thread .github/workflows/ci.yml
Comment on lines +28 to +29
- name: Test
run: swift test
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Let's start running tests in CI.

I couldn't get Linux tests to pass, but I got it close. Maybe we can discuss later today.

@mbrandonw mbrandonw mentioned this pull request Jan 26, 2026
@mbrandonw mbrandonw merged commit db8ce9e into main Jan 28, 2026
3 checks passed
@mbrandonw mbrandonw deleted the tests branch January 28, 2026 18:05
@mbrandonw mbrandonw restored the tests branch January 28, 2026 18:13
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.

2 participants