-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
Verify if tests communicate with just spawned dev server #479
Changes from 2 commits
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 |
---|---|---|
|
@@ -21,12 +21,17 @@ struct CartonDevPluginCommand: CommandPlugin { | |
var product: String? | ||
var release: Bool | ||
var verbose: Bool | ||
var pid: String? | ||
|
||
static func parse(from extractor: inout ArgumentExtractor) throws -> Options { | ||
let product = extractor.extractOption(named: "product").last | ||
let release = extractor.extractFlag(named: "release") | ||
let verbose = extractor.extractFlag(named: "verbose") | ||
return Options(product: product, release: release != 0, verbose: verbose != 0) | ||
let pid = extractor.extractOption(named: "pid").last | ||
return Options( | ||
product: product, release: release != 0, verbose: verbose != 0, | ||
pid: pid | ||
) | ||
} | ||
} | ||
|
||
|
@@ -82,19 +87,19 @@ struct CartonDevPluginCommand: CommandPlugin { | |
let buildRequestPipe = try createFifo(hint: "build-request", directory: tempDirectory) | ||
let buildResponsePipe = try createFifo(hint: "build-response", directory: tempDirectory) | ||
|
||
let frontend = try makeCartonFrontendProcess( | ||
context: context, | ||
arguments: [ | ||
"dev", | ||
"--main-wasm-path", productArtifact.path.string, | ||
"--build-request", buildRequestPipe, | ||
"--build-response", buildResponsePipe, | ||
] | ||
+ resourcesPaths.flatMap { ["--resources", $0.string] } | ||
+ pathsToWatch.flatMap { ["--watch-path", $0] } | ||
+ (options.verbose ? ["--verbose"] : []) | ||
+ extractor.remainingArguments | ||
) | ||
var args: [String] = [ | ||
"dev", | ||
"--main-wasm-path", productArtifact.path.string, | ||
"--build-request", buildRequestPipe, | ||
"--build-response", buildResponsePipe | ||
] | ||
args += (options.pid.map { ["--pid", $0] } ?? []) | ||
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. パッチとしての挙動の変更は |
||
args += resourcesPaths.flatMap { ["--resources", $0.string] } | ||
args += pathsToWatch.flatMap { ["--watch-path", $0] } | ||
args += (options.verbose ? ["--verbose"] : []) | ||
args += extractor.remainingArguments | ||
|
||
let frontend = try makeCartonFrontendProcess(context: context, arguments: args) | ||
frontend.forwardTerminationSignals() | ||
|
||
try frontend.run() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,11 +20,16 @@ struct CartonTestPluginCommand: CommandPlugin { | |
struct Options { | ||
var environment: Environment | ||
var prebuiltTestBundlePath: String? | ||
var pid: String? | ||
|
||
static func parse(from extractor: inout ArgumentExtractor) throws -> Options { | ||
let environment = try Environment.parse(from: &extractor) | ||
let prebuiltTestBundlePath = extractor.extractOption(named: "prebuilt-test-bundle-path").first | ||
return Options(environment: environment, prebuiltTestBundlePath: prebuiltTestBundlePath) | ||
let pid = extractor.extractOption(named: "pid").last | ||
return Options( | ||
environment: environment, prebuiltTestBundlePath: prebuiltTestBundlePath, | ||
pid: pid | ||
) | ||
} | ||
} | ||
|
||
|
@@ -98,17 +103,18 @@ struct CartonTestPluginCommand: CommandPlugin { | |
package: context.package | ||
) | ||
|
||
let frontendArguments = | ||
[ | ||
"test", | ||
"--prebuilt-test-bundle-path", testProductArtifactPath, | ||
"--environment", options.environment.rawValue, | ||
"--plugin-work-directory", context.pluginWorkDirectory.string | ||
] | ||
+ resourcesPaths.flatMap { | ||
["--resources", $0.string] | ||
} + extractor.remainingArguments | ||
let frontend = try makeCartonFrontendProcess(context: context, arguments: frontendArguments) | ||
var args: [String] = [ | ||
"test", | ||
"--prebuilt-test-bundle-path", testProductArtifactPath, | ||
"--environment", options.environment.rawValue, | ||
"--plugin-work-directory", context.pluginWorkDirectory.string | ||
] | ||
args += (options.pid.map { ["--pid", $0] } ?? []) | ||
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. ここで |
||
args += resourcesPaths.flatMap { | ||
["--resources", $0.string] | ||
} | ||
args += extractor.remainingArguments | ||
let frontend = try makeCartonFrontendProcess(context: context, arguments: args) | ||
try frontend.checkRun(printsLoadingMessage: false, forwardExit: true) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
struct CartonCoreError: Error & CustomStringConvertible { | ||
init(_ description: String) { | ||
public struct CartonCoreError: Error & CustomStringConvertible { | ||
public init(_ description: String) { | ||
self.description = description | ||
} | ||
var description: String | ||
public var description: String | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,6 +54,8 @@ func derivePackageCommandArguments( | |
let pluginArguments: [String] = ["plugin"] | ||
var cartonPluginArguments: [String] = extraArguments | ||
|
||
let pid = ProcessInfo.processInfo.processIdentifier | ||
|
||
switch subcommand { | ||
case "bundle": | ||
packageArguments += ["--disable-sandbox"] | ||
|
@@ -64,6 +66,7 @@ func derivePackageCommandArguments( | |
cartonPluginArguments = ["--output", "Bundle"] + cartonPluginArguments | ||
case "dev": | ||
packageArguments += ["--disable-sandbox"] | ||
cartonPluginArguments += ["--pid", pid.description] | ||
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. driverがpluginを起動するときに、 |
||
case "test": | ||
// 1. Ask the plugin process to generate the build command based on the given options | ||
let commandFile = try makeTemporaryFile(prefix: "test-build") | ||
|
@@ -92,6 +95,7 @@ func derivePackageCommandArguments( | |
|
||
// "--environment browser" launches a http server | ||
packageArguments += ["--disable-sandbox"] | ||
cartonPluginArguments += ["--pid", pid.description] | ||
default: break | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,6 +113,8 @@ struct CartonFrontendDevCommand: AsyncParsableCommand { | |
) | ||
var mainWasmPath: String | ||
|
||
@Option(name: .long) var pid: Int32? | ||
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. Could you hide the internal option from the help by 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. そういうのがあるんですね。対応しました。 |
||
|
||
static let configuration = CommandConfiguration( | ||
commandName: "dev", | ||
abstract: "Watch the current directory, host the app, rebuild on change." | ||
|
@@ -169,6 +171,7 @@ struct CartonFrontendDevCommand: AsyncParsableCommand { | |
}, | ||
resourcesPaths: resources, | ||
entrypoint: Self.entrypoint, | ||
pid: pid, | ||
terminal: terminal | ||
) | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
import CartonCore | ||
import CartonHelpers | ||
import Foundation | ||
import Logging | ||
|
@@ -106,6 +107,42 @@ public actor Server { | |
hasher.combine(ObjectIdentifier(self)) | ||
} | ||
} | ||
|
||
public static let serverName = "carton dev server" | ||
|
||
public struct ServerNameField: CustomStringConvertible { | ||
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.
|
||
public init( | ||
name: String = serverName, | ||
version: String = cartonVersion, | ||
pid: Int32 | ||
) { | ||
self.name = name | ||
self.version = version | ||
self.pid = pid | ||
} | ||
|
||
public var name: String | ||
public var version: String | ||
public var pid: Int32 | ||
|
||
public var description: String { | ||
"\(name)/\(version) (PID \(pid))" | ||
} | ||
|
||
private static let regex = #/([\w ]+)/([\w\.]+) \(PID (\d+)\)/# | ||
|
||
public static func parse(_ string: String) throws -> ServerNameField { | ||
guard let m = try regex.wholeMatch(in: string), | ||
let pid = Int32(m.output.3) else { | ||
throw CartonCoreError("invalid server name: \(string)") | ||
} | ||
|
||
let name = String(m.output.1) | ||
let version = String(m.output.2) | ||
return ServerNameField(name: name, version: version, pid: pid) | ||
} | ||
} | ||
|
||
/// Used for decoding `Event` values sent from the WebSocket client. | ||
private let decoder = JSONDecoder() | ||
|
||
|
@@ -131,6 +168,8 @@ public actor Server { | |
|
||
private let configuration: Configuration | ||
|
||
private let serverName: ServerNameField | ||
|
||
public struct Configuration { | ||
let builder: BuilderProtocol? | ||
let mainWasmPath: AbsolutePath | ||
|
@@ -141,6 +180,7 @@ public actor Server { | |
let customIndexPath: AbsolutePath? | ||
let resourcesPaths: [String] | ||
let entrypoint: Entrypoint | ||
let pid: Int32? | ||
let terminal: InteractiveWriter | ||
|
||
public init( | ||
|
@@ -153,6 +193,7 @@ public actor Server { | |
customIndexPath: AbsolutePath?, | ||
resourcesPaths: [String], | ||
entrypoint: Entrypoint, | ||
pid: Int32?, | ||
terminal: InteractiveWriter | ||
) { | ||
self.builder = builder | ||
|
@@ -164,6 +205,7 @@ public actor Server { | |
self.customIndexPath = customIndexPath | ||
self.resourcesPaths = resourcesPaths | ||
self.entrypoint = entrypoint | ||
self.pid = pid | ||
self.terminal = terminal | ||
} | ||
|
||
|
@@ -184,6 +226,9 @@ public actor Server { | |
self.localURL = localURL | ||
watcher = nil | ||
self.configuration = configuration | ||
self.serverName = ServerNameField( | ||
pid: configuration.pid ?? ProcessInfo.processInfo.processIdentifier | ||
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. 指定がなければ自身のプロセス番号を広告します。 |
||
) | ||
|
||
guard let builder = configuration.builder else { | ||
return | ||
|
@@ -293,7 +338,8 @@ public actor Server { | |
mainWasmPath: configuration.mainWasmPath, | ||
customIndexPath: configuration.customIndexPath, | ||
resourcesPaths: configuration.resourcesPaths, | ||
entrypoint: configuration.entrypoint | ||
entrypoint: configuration.entrypoint, | ||
serverName: serverName.description | ||
) | ||
let channel = try await ServerBootstrap(group: group) | ||
// Specify backlog and enable SO_REUSEADDR for the server itself | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
import ArgumentParser | ||
import XCTest | ||
import CartonHelpers | ||
import CartonKit | ||
|
||
#if canImport(FoundationNetworking) | ||
import FoundationNetworking | ||
|
@@ -91,8 +92,10 @@ func swiftRunProcess( | |
stdout: { (chunk) in | ||
outputBuffer += chunk | ||
stdoutStream.write(sequence: chunk) | ||
stdoutStream.flush() | ||
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. ここxctest の実行時に全然出力が渡ってきませんでした。 |
||
}, stderr: { (chunk) in | ||
stderrStream.write(sequence: chunk) | ||
stderrStream.flush() | ||
}, | ||
redirectStderr: false | ||
) | ||
|
@@ -134,3 +137,18 @@ func fetchWebContent(at url: URL, timeout: Duration) async throws -> (response: | |
|
||
return (response: response, body: body) | ||
} | ||
|
||
func checkServerNameField(response: HTTPURLResponse, expectedPID: Int32) throws { | ||
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. レスポンスヘッダの中のPIDを検証するユーティリティ関数です。 |
||
guard let string = response.value(forHTTPHeaderField: "Server") else { | ||
throw CommandTestError("no Server header") | ||
} | ||
let field = try Server.ServerNameField.parse(string) | ||
|
||
guard field.name == Server.serverName else { | ||
throw CommandTestError("invalid server name: \(field)") | ||
} | ||
|
||
guard field.pid == expectedPID else { | ||
throw CommandTestError("Expected PID \(expectedPID) but got PID \(field.pid).") | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,6 +82,7 @@ final class DevCommandTests: XCTestCase { | |
|
||
let (response, data) = try await fetchDevServerWithRetry(at: try URL(string: url).unwrap("url")) | ||
XCTAssertEqual(response.statusCode, 200, "Response was not ok") | ||
try checkServerNameField(response: response, expectedPID: process.process.processID) | ||
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. 通信相手はちゃんとテスト対象のプロセスだったのか検証します。 |
||
|
||
let expectedHtml = """ | ||
<!DOCTYPE html> | ||
|
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.
ここ、巨大なリテラルと演算子が混ざっていて型推論が壊れやすいです。
正しいコードを書いていればいいですが、
作業中に間違ったコードを書いたりした時に IDEサポートがなくなってしまうのでやりづらいです。
式を分割して安定させます。