Repository: google/swift-benchmark
Pull request #44 (2020-06-15 13:48:35)

Allow for 0 --warmup-iterations
This change makes treatment of absent or 0 warmup iterations more consistent. There is no need to differentiate between unset warmup iterations setting and warmup iterations explicitly set to 0.


Patch:
diff --git a/Sources/Benchmark/BenchmarkArguments.swift b/Sources/Benchmark/BenchmarkArguments.swift
index ae67b09..5c3fc90 100644
--- a/Sources/Benchmark/BenchmarkArguments.swift
+++ b/Sources/Benchmark/BenchmarkArguments.swift
@@ -75,8 +75,9 @@ public struct BenchmarkArguments: ParsableArguments {
         if iterations != nil && iterations! <= 0 {
             throw ValidationError(positiveNumberError(flag: "--iterations", of: "integer"))
         }
-        if warmupIterations != nil && warmupIterations! <= 0 {
-            throw ValidationError(positiveNumberError(flag: "--warmup-iterations", of: "integer"))
+        if warmupIterations != nil && warmupIterations! < 0 {
+            throw ValidationError(
+                nonNegativeNumberError(flag: "--warmup-iterations", of: "integer"))
         }
         if maxIterations != nil && maxIterations! <= 0 {
             throw ValidationError(positiveNumberError(flag: "--max-iterations", of: "integer"))
@@ -99,4 +100,8 @@ public struct BenchmarkArguments: ParsableArguments {
     func positiveNumberError(flag: String, of type: String) -> String {
         return "Value provided via \(flag) must be a positive \(type)."
     }
+
+    func nonNegativeNumberError(flag: String, of type: String) -> String {
+        return "Value provided via \(flag) must be a non-negative \(type)."
+    }
 }
diff --git a/Sources/Benchmark/BenchmarkRunner.swift b/Sources/Benchmark/BenchmarkRunner.swift
index 045171b..7dd49ac 100644
--- a/Sources/Benchmark/BenchmarkRunner.swift
+++ b/Sources/Benchmark/BenchmarkRunner.swift
@@ -54,8 +54,9 @@ public struct BenchmarkRunner {
         let totalStart = now()
 
         var warmupState: BenchmarkState? = nil
-        if let n = settings.warmupIterations {
-            warmupState = doNIterations(n, benchmark: benchmark, suite: suite, settings: settings)
+        if settings.warmupIterations > 0 {
+            warmupState = doNIterations(
+                settings.warmupIterations, benchmark: benchmark, suite: suite, settings: settings)
         }
 
         var state: BenchmarkState
diff --git a/Sources/Benchmark/BenchmarkSetting.swift b/Sources/Benchmark/BenchmarkSetting.swift
index 7cb60cc..e6b1ad7 100644
--- a/Sources/Benchmark/BenchmarkSetting.swift
+++ b/Sources/Benchmark/BenchmarkSetting.swift
@@ -125,8 +125,12 @@ public struct BenchmarkSettings {
     }
 
     /// Convenience accessor for WarmupIterations setting.
-    public var warmupIterations: Int? {
-        return self[WarmupIterations.self]?.value
+    public var warmupIterations: Int {
+        if let value = self[WarmupIterations.self]?.value {
+            return value
+        } else {
+            return 0
+        }
     }
 
     /// Convenience accessor for the Filter setting.


Test patch:
diff --git a/Tests/BenchmarkTests/BenchmarkCommandTests.swift b/Tests/BenchmarkTests/BenchmarkCommandTests.swift
index d4e2080..121ee93 100644
--- a/Tests/BenchmarkTests/BenchmarkCommandTests.swift
+++ b/Tests/BenchmarkTests/BenchmarkCommandTests.swift
@@ -73,10 +73,16 @@ final class BenchmarkCommandTests: XCTestCase {
         }
     }
 
-    func testParseZeroWarmupIterationsError() throws {
-        AssertFailsToParse(
-            ["--warmup-iterations", "0", "--allow-debug-build"],
-            with: "Value provided via --warmup-iterations must be a positive integer.")
+    func testParseZeroWarmupIterations() throws {
+        AssertParse(["--warmup-iterations", "0", "--allow-debug-build"]) { settings in
+            XCTAssertEqual(settings.warmupIterations, 0)
+        }
+    }
+
+    func testParseNoWarmupIterations() throws {
+        AssertParse(["--allow-debug-build"]) { settings in
+            XCTAssertEqual(settings.warmupIterations, 0)
+        }
     }
 
     func testParseMaxIterations() throws {
@@ -87,7 +93,7 @@ final class BenchmarkCommandTests: XCTestCase {
 
     func testParseZeroMaxIterationsError() throws {
         AssertFailsToParse(
-            ["--warmup-iterations", "0", "--allow-debug-build"],
+            ["--max-iterations", "0", "--allow-debug-build"],
             with: "Value provided via --max-iterations must be a positive integer.")
     }
 
@@ -110,9 +116,10 @@ final class BenchmarkCommandTests: XCTestCase {
         ("testParseIterations", testParseIterations),
         ("testParseZeroIterationsError", testParseZeroIterationsError),
         ("testParseWarmupIterations", testParseWarmupIterations),
-        ("testParseZeroWarmupIterationsError", testParseZeroWarmupIterationsError),
+        ("testParseZeroWarmupIterations", testParseZeroWarmupIterations),
+        ("testParseNoWarmupIterations", testParseNoWarmupIterations),
         ("testParseMaxIterations", testParseWarmupIterations),
-        ("testParseZeroMaxIterationsError", testParseZeroWarmupIterationsError),
+        ("testParseZeroMaxIterationsError", testParseZeroMaxIterationsError),
         ("testParseMinTime", testParseMinTime),
         ("testParseZeroMinTimeError", testParseZeroMinTimeError),
     ]