Add a modeled command-line for better checking #330
Conversation
Swiftly has many different commands that it invokes in different parts of its code base, supporting tooling, and tests. This list includes tar, git, pkgutil, and dscl among oseveral others. For the most part these tools are called using arrays that are manually assembled at the call site. In order to exercise that callsite in tests the test must necessarily become "medium" size because it involves integration testing with that tool, instead of "small", which is where most tests should be. A small test of that array would verify that the length has the correct arguments, number, spelling, and order. Also, there are various types of string interpolation in these arrays, which limit the type-safety. Introduce a new way of modeling command-line invocations using Swift types. Commands and subcommands are modeled using structs. Use functions to assemble the structs, and methods to assemble structs for subcommands. Arguments become function arguments, with default values if they are optional. Flags, and options will be described using enum cases with carrying values for options. Some commands are runnable, while others are not. Introduce a Runnable protocol that marks runnable commands, and provides a default implementation of a run() method to run the assembled command. Other commands can produce output. Introduce an Output protocol and default implementation that runs the command and receives the output as a string that the callsite can use. Some callsites make decisions based on whether a tool is present or not. Provide a convenient way to verify whether a command exists on the system. Use this mechanism to skip tests when they are run in an environment that doesn't have the tool. Pick one command, dscl, to use with the new modeled command-line. Write small tests that don't invoke the tool. Instead they verify the configuration that is produced with different typical inputs from swiftly. Add a single medium size test that runs dscl from a modeled command, enabled only if dscl exists on the system. Add tags for medium, and large tests. Mark existing large tests, which is the HTTPClientTests. Leave all of the remaining tests as untagged, (i.e. small). Mark the CommandLineTest that invokes the dscl tool as medium.
|
@swift-ci test macOS |
Sources/SwiftlyCore/Commands.swift
Outdated
| import SystemPackage | ||
|
|
||
| // Directory Service command line utility for macOS | ||
| public enum SystemCommand { |
There was a problem hiding this comment.
nit: define SystemCommand as an empty enum and build each command as a standalone extension on that enum
Sources/SwiftlyCore/Commands.swift
Outdated
|
|
||
| return Configuration( | ||
| executable: self.executable, | ||
| arguments: .init(args), |
There was a problem hiding this comment.
nit: favour using the explicit initializer (Arguments) explicit over using .init
| } | ||
|
|
||
| internal enum StringOrRawBytes: Sendable, Hashable { | ||
| case string(String) |
There was a problem hiding this comment.
This enum seems to be unnecessary w/ only one case?
There was a problem hiding this comment.
This code comes from the new subprocess package, which unfortunately is targeting a newer macOS requirement than swiftly at the moment (should be fixed soon). This is a duplicate of the relevant data structures from there so that it's aligned with it once it's usable here.
| } | ||
|
|
||
| extension Executable { | ||
| public func exists() async throws -> Bool { |
There was a problem hiding this comment.
This method doesn't look to be used. I noticed because checking for existence of something often leads to a potential race condition where the thing may disappear in between checking for its existence and its usage. Its often better to just try and use the resource and handle the failure if it doesn't work.
There was a problem hiding this comment.
This is used as the condition on a the medium sized test, so that the test can be skipped. I suppose I could force that check to actually attempt to run a sample command-line, but this seemed better. I'll move this extension over to the SwiftlyTests module since I generally agree with your assessment. It's just a testing concern.
There was a problem hiding this comment.
Perhaps we could hide this method behind @_spi(Testing) to indicate it is exposed only for test purposes?
There was a problem hiding this comment.
Thanks, I've moved it over to the testing target.
|
This looks cool! I like the strong-typedness and the fluent API style. |
|
@swift-ci test macOS |
|
@swift-ci test macOS |
Swiftly has many different commands that it invokes in different parts of its code base, supporting tooling, and tests. This list includes tar, git, pkgutil, and dscl among several others.
For the most part these tools are called using arrays that are manually assembled at the call site. In order to exercise that callsite in tests the test must necessarily become "medium" size because it involves integration testing with that tool, instead of "small", which is where most tests should be. A small test of that array would verify that the length has the correct arguments, number, spelling, and order. Also, there are various types of string interpolation in these arrays, which limit the type-safety.
Introduce a new way of modeling command-line invocations using Swift types. Commands and subcommands are modeled using structs. Use functions to assemble the structs, and methods to assemble structs for subcommands. Arguments become function arguments, with default values if they are optional. Flags, and options will be described using enum cases with carrying values for options.
Some commands are runnable, while others are not. Introduce a Runnable protocol that marks runnable commands, and provides a default implementation of a run() method to run the assembled command.
Other commands can produce output. Introduce an Output protocol and default implementation that runs the command and receives the output as a string that the callsite can use.
Some callsites make decisions based on whether a tool is present or not. Provide a convenient way to verify whether a command exists on the system. Use this mechanism to skip tests when they are run in an environment that doesn't have the tool.
Pick one command, dscl, to use with the new modeled command-line. Write small tests that don't invoke the tool. Instead they verify the configuration that is produced with different typical inputs from swiftly. Add a single medium size test that runs dscl from a modeled command, enabled only if dscl exists on the system.
Add tags for medium, and large tests. Mark existing large tests, which is the HTTPClientTests. Leave all of the remaining tests as untagged, (i.e. small). Mark the CommandLineTest that invokes the dscl tool as medium.