Skip to content
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

Check the output of wasm in dev subcommand test #473

Merged
merged 4 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,11 @@ struct CartonFrontendDevCommand: AsyncParsableCommand {
)
let localURL = try await server.start()
if !skipAutoOpen {
openInSystemBrowser(url: localURL)
do {
try openInSystemBrowser(url: localURL)
} catch {
terminal.write("open browser failed: \(error)", inColor: .red)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

従来通り、起動しなくても処理は続ける

}
}
try await server.waitUntilStop()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ struct BrowserTestRunner: TestRunner {
try await client.goto(url: localURL)
} else {
disposer = {}
openInSystemBrowser(url: localURL)
try openInSystemBrowser(url: localURL)
}
let hadError = try await server.waitUntilTestFinished()
try await disposer()
Expand Down
39 changes: 24 additions & 15 deletions Sources/CartonKit/Server/Server.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,18 @@ public protocol BuilderProtocol {
}

public actor Server {
public enum Error: Swift.Error & CustomStringConvertible {
case invalidURL(String)
case noOpenBrowserPlatform

public var description: String {
switch self {
case .invalidURL(let string): "Invalid URL: \(string)"
case .noOpenBrowserPlatform: "This platform cannot launch a browser."
}
}
}

final class Connection: Hashable {
let channel: Channel

Expand Down Expand Up @@ -105,8 +117,8 @@ public actor Server {

private var serverChannel: (any Channel)!

/// Local URL of this server, `https://128.0.0.1:8080/` by default.
private let localURL: String
/// Local URL of this server, `https://127.0.0.1:8080/` by default.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

localhostは127です

private let localURL: URL
Copy link
Contributor Author

Choose a reason for hiding this comment

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

早めにURL型にします


/// Whether a build that could be triggered by this server is currently running.
private var isBuildCurrentlyRunning = false
Expand Down Expand Up @@ -156,7 +168,11 @@ public actor Server {
public init(
_ configuration: Configuration
) async throws {
localURL = "http://\(configuration.host):\(configuration.port)/"
let localURLString = "http://\(configuration.host):\(configuration.port)/"
guard let localURL = URL(string: localURLString) else {
throw Error.invalidURL(localURLString)
}
self.localURL = localURL
watcher = nil
self.configuration = configuration

Expand Down Expand Up @@ -222,7 +238,7 @@ public actor Server {
connections.remove(connection)
}

public func start() async throws -> String {
public func start() async throws -> URL {
let group = MultiThreadedEventLoopGroup.singleton
let upgrader = NIOWebSocketServerUpgrader(
maxFrameSize: Int(UInt32.max),
Expand Down Expand Up @@ -421,25 +437,18 @@ extension Server {
}

/// Attempts to open the specified URL string in system browser on macOS and Linux.
/// - Returns: true if launching command returns successfully.
@discardableResult
public func openInSystemBrowser(url: String) -> Bool {
public func openInSystemBrowser(url: URL) throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stringを受け取ってBoolを返していましたが、
URLを受け取って失敗したら例外を投げるようにします。

引数については型を活用します。
エラーについてですが、例外の方が情報が多いです。

おそらく元々の実装はブラウザオープンに失敗しても動作を止めないことを想定していますが、
それは関数呼び出し側の都合で、必要ならcatchすれば良いです。

#if os(macOS)
let openCommand = "open"
#elseif os(Linux)
let openCommand = "xdg-open"
#else
return false
throw Server.Error.noOpenBrowserPlatform
#endif
let process = Process(
arguments: [openCommand, url],
arguments: [openCommand, url.absoluteString],
outputRedirection: .none,
startNewProcessGroup: true
)
do {
try process.launch()
return true
} catch {
return false
}
try process.launch()
}
4 changes: 2 additions & 2 deletions Sources/WebDriverClient/WebDriverClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,13 @@ public struct WebDriverClient {
return try encoder.encode(body)
}

public func goto(url: String) async throws {
public func goto(url: URL) async throws {
struct Request: Encodable {
let url: String
}
var request = URLRequest(url: URL(string: makeSessionURL("url"))!)
request.httpMethod = "POST"
request.httpBody = try Self.makeRequestBody(Request(url: url))
request.httpBody = try Self.makeRequestBody(Request(url: url.absoluteString))
request.addValue("carton", forHTTPHeaderField: "User-Agent")
_ = try await client.data(for: request)
}
Expand Down
30 changes: 29 additions & 1 deletion Tests/CartonCommandTests/FrontendDevServerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,31 @@ final class FrontendDevServerTests: XCTestCase {

try await Process.run(["swift", "build", "--target", "carton-frontend"], terminal)

var gotHelloStdout = false
var gotHelloStderr = false

let devServer = Process(
arguments: [
"swift", "run", "carton-frontend", "dev",
"--skip-auto-open", "--verbose",
"--main-wasm-path", wasmFile.pathString,
"--resources", resourcesDir.pathString
]
],
outputRedirection: .stream(
stdout: { (chunk) in
let string = String(decoding: chunk, as: UTF8.self)

if string.contains("stdout: hello stdout") {
gotHelloStdout = true
}
if string.contains("stderr: hello stderr") {
gotHelloStderr = true
}

terminal.write(string)
}, stderr: { (_) in },
redirectStderr: true
)
)
try devServer.launch()
defer {
Expand Down Expand Up @@ -84,6 +102,16 @@ final class FrontendDevServerTests: XCTestCase {
let expected = try String(contentsOf: resourcesDir.appending(component: name).asURL)
XCTAssertEqual(styleCss, expected)
}

try openInSystemBrowser(url: host)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think open families work on CI. We should use WebDriver or something like headless solutions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そうですね。
$ carton test で使われている実装を再利用するようにコードを組んでみます。


for _ in 0..<30 {
try await Task.sleep(for: .seconds(1))
if gotHelloStdout, gotHelloStderr { break }
}

XCTAssertTrue(gotHelloStdout)
XCTAssertTrue(gotHelloStderr)
}

private func fetchBinary(
Expand Down
2 changes: 1 addition & 1 deletion Tests/WebDriverClientTests/WebDriverClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ final class WebDriverClientTests: XCTestCase {
let client = try await WebDriverClient.newSession(
endpoint: checkRemoteURL(), httpClient: .shared
)
try await client.goto(url: "https://example.com")
try await client.goto(url: URL(string: "https://github.com/swiftwasm/carton")!)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

一応自分たちが管理してるURLの方が良いと思います。

Copy link
Member

Choose a reason for hiding this comment

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

The example.com is managed by ICANN. It should be more reliable than GitHub IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

なるほど、OKです。戻します。

try await client.closeSession()
}
}
Loading