Skip to content

Commit 50575b1

Browse files
committed
[benchmark][Gardening] Introduce PartialTestConfig
Moving towards immutable TestConfig. The pattern used is inspired by the`PartialBenchmarkConfig` from [criterion.rs](https://github.com/japaric/criterion.rs/blob/master/src/benchmark.rs). The idea is to have a temporary mutable struct which collects the command line arguments and is filled by the “parser” (which will be extracted later). The partial configuration is used to initialize the immutable `TestConfig`. The optionality of the command line parameters is represented by the corresponding properties being `Optional`. (Accidentaly, all our arguments are optional… but that’s beside the point.) Null coalescing operator is used to fill in the defaults during initialization. For some reason, the compiler found `optionalArg` calls for `tags` and `skip-tags` ambiguous, when types changed to `Set<BenchmarkCategory>?`, which was resolved by providing fully qualified key paths for them (`\PartialTestConfig.tags`).
1 parent b76c946 commit 50575b1

File tree

1 file changed

+45
-15
lines changed

1 file changed

+45
-15
lines changed

benchmark/utils/DriverUtils.swift

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,44 +32,56 @@ enum TestAction {
3232

3333
struct TestConfig {
3434
/// The delimiter to use when printing output.
35-
var delim: String = ","
35+
let delim: String
3636

3737
/// The filters applied to our test names.
38-
var filters = [String]()
38+
let filters: [String]
3939

4040
/// The tags that we want to run
41-
var tags = Set<BenchmarkCategory>()
41+
let tags: Set<BenchmarkCategory>
4242

4343
/// Tests tagged with any of these will not be executed
44-
var skipTags: Set<BenchmarkCategory> = [.unstable, .skip]
44+
let skipTags: Set<BenchmarkCategory>
4545

4646
/// The scalar multiple of the amount of times a test should be run. This
4747
/// enables one to cause tests to run for N iterations longer than they
4848
/// normally would. This is useful when one wishes for a test to run for a
4949
/// longer amount of time to perform performance analysis on the test in
5050
/// instruments.
51-
var iterationScale: Int = 1
51+
let iterationScale: Int
5252

5353
/// If we are asked to have a fixed number of iterations, the number of fixed
5454
/// iterations.
55-
var fixedNumIters: UInt = 0
55+
let fixedNumIters: UInt
5656

5757
/// The number of samples we should take of each test.
58-
var numSamples: Int = 1
58+
let numSamples: Int
5959

6060
/// Is verbose output enabled?
61-
var verbose: Bool = false
61+
let verbose: Bool
6262

6363
/// After we run the tests, should the harness sleep to allow for utilities
6464
/// like leaks that require a PID to run on the test harness.
65-
var afterRunSleep: Int?
65+
let afterRunSleep: Int?
6666

6767
/// The list of tests to run.
6868
var tests = [(index: String, info: BenchmarkInfo)]()
6969

70-
var action: TestAction = .run
70+
let action: TestAction
7171

7272
init() throws {
73+
74+
struct PartialTestConfig {
75+
var delim: String?
76+
var filters: [String]?
77+
var tags, skipTags: Set<BenchmarkCategory>?
78+
var iterationScale, numSamples, afterRunSleep: Int?
79+
var fixedNumIters: UInt?
80+
var verbose: Bool?
81+
var action: TestAction?
82+
}
83+
var c = PartialTestConfig()
84+
7385
let validOptions = [
7486
"--iter-scale", "--num-samples", "--num-iters",
7587
"--verbose", "--delim", "--list", "--sleep",
@@ -87,21 +99,27 @@ struct TestConfig {
8799
argument: String? = nil
88100
) throws -> T {
89101
if let t = try parse(value) { return t }
102+
var type = "\(T.self)"
103+
if type.starts(with: "Optional<") {
104+
let s = type.index(after: type.index(of:"<")!)
105+
let e = type.index(before: type.endIndex) // ">"
106+
type = String(type[s..<e]) // strip Optional< >
107+
}
90108
throw ArgumentError.invalidType(
91-
value: value, type: "\(T.self)", argument: argument)
109+
value: value, type: type, argument: argument)
92110
}
93111

94112
func optionalArg<T>(
95113
_ name: String,
96-
_ property: WritableKeyPath<TestConfig, T>,
114+
_ property: WritableKeyPath<PartialTestConfig, T>,
97115
defaultValue: T? = nil,
98116
parser parse: (String) throws -> T? = { _ in nil }
99117
) throws {
100118
if let value = benchArgs.optionalArgsMap[name] {
101119
guard !value.isEmpty || defaultValue != nil
102120
else { throw ArgumentError.missingValue(name) }
103121

104-
self[keyPath: property] = (value.isEmpty)
122+
c[keyPath: property] = (value.isEmpty)
105123
? defaultValue!
106124
: try checked(parse, value, argument:name)
107125
}
@@ -121,12 +139,24 @@ struct TestConfig {
121139
try optionalArg("--num-samples", \.numSamples) { Int($0) }
122140
try optionalArg("--verbose", \.verbose, defaultValue: true)
123141
try optionalArg("--delim", \.delim) { $0 }
124-
try optionalArg("--tags", \.tags, parser: tags)
125-
try optionalArg("--skip-tags", \.skipTags, defaultValue: [], parser: tags)
142+
try optionalArg("--tags", \PartialTestConfig.tags, parser: tags)
143+
try optionalArg("--skip-tags", \PartialTestConfig.skipTags,
144+
defaultValue: [], parser: tags)
126145
try optionalArg("--sleep", \.afterRunSleep) { Int($0) }
127146
try optionalArg("--list", \.action, defaultValue: .listTests)
128147
try optionalArg("--help", \.action, defaultValue: .help(validOptions))
129148

149+
// Configure from the command line arguments, filling in the defaults.
150+
delim = c.delim ?? ","
151+
self.tags = c.tags ?? []
152+
skipTags = c.skipTags ?? [.unstable, .skip]
153+
iterationScale = c.iterationScale ?? 1
154+
fixedNumIters = c.fixedNumIters ?? 0
155+
numSamples = c.numSamples ?? 1
156+
verbose = c.verbose ?? false
157+
afterRunSleep = c.afterRunSleep
158+
action = c.action ?? .run
159+
// TODO: filters, tests
130160
}
131161

132162
mutating func findTestsToRun() {

0 commit comments

Comments
 (0)