-
Notifications
You must be signed in to change notification settings - Fork 54
Add a test for a custom OutputProtocol-conforming type #235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
cdf0d61
bb9c5d8
69bdd32
14b1d9d
6cf9d97
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This source file is part of the Swift.org open source project | ||
| // | ||
| // Copyright (c) 2026 Apple Inc. and the Swift project authors | ||
| // Licensed under Apache License v2.0 with Runtime Library Exception | ||
| // | ||
| // See https://swift.org/LICENSE.txt for license information | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| import Testing | ||
| import Subprocess | ||
|
|
||
| #if canImport(Darwin) | ||
| import Foundation | ||
| #else | ||
| import FoundationEssentials | ||
| #endif | ||
|
|
||
| @Suite(.serialized) | ||
| struct ProtocolConformanceTests { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (nit) Any particular reason to create a new test file instead of just reusing IntegrationTests?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm used to having a struct for one particular kind of test, so I created another one here. Honestly I didn't look closely what IntegrationTest contains. I can move back if you prefer
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh not at all. It's probably a good idea to split them up anyways. |
||
|
|
||
| @Test func customOutputJSON() async throws { | ||
| struct JSONOutput<T: Decodable & Sendable>: OutputProtocol { | ||
| typealias OutputType = T | ||
|
|
||
| func output(from span: RawSpan) throws -> T { | ||
| try span.withUnsafeBytes { buffer in | ||
| let data = Data(bytes: buffer.baseAddress!, count: buffer.count) | ||
| return try JSONDecoder().decode(T.self, from: data) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| struct Item: Codable, Sendable, Equatable { | ||
| let title: String | ||
| } | ||
|
|
||
| let json = #"{"title":"Hello from Subprocess"}"# | ||
|
|
||
| #if os(Windows) | ||
| let result = try await Subprocess.run( | ||
| .name("powershell.exe"), | ||
| arguments: ["-Command", "Write-Output '\(json)'"], | ||
| output: JSONOutput<Item>(), | ||
| error: .discarded | ||
| ) | ||
| #else | ||
| let result = try await Subprocess.run( | ||
| .name("echo"), | ||
| arguments: [json], | ||
| output: JSONOutput<Item>(), | ||
| error: .discarded | ||
| ) | ||
| #endif | ||
|
|
||
| #expect(result.terminationStatus.isSuccess) | ||
| #expect(result.standardOutput == Item(title: "Hello from Subprocess")) | ||
| #expect(result.standardOutput.title == "Hello from Subprocess") | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(not a issue) This TODO was more about switching to a String initializer that takes a Span directly. I don't think that API is ready yet unfortunately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I know the intention. I was looking for it so eagerly the other day too. For now at least this change stops us from potentially reallocating the array... still need a Span one though unfortunately.