Repository: google/swift-benchmark
Pull request #46 (2020-06-15 17:59:06)

Use . as separator instead of : in benchmark names
Based on initial use cases, we seem to lean towards non-whitespace delimited names for benchmark suites and benchmarks. Using `.` as a name separator (i.e., `Suite.benchmark` instead of `Suite: benchmark`) seems to look better and is less error-prone while integrating with other tools which might require non-whitespace delimited names.


Patch:
diff --git a/Sources/Benchmark/BenchmarkArguments.swift b/Sources/Benchmark/BenchmarkArguments.swift
index 7f97828..d78e164 100644
--- a/Sources/Benchmark/BenchmarkArguments.swift
+++ b/Sources/Benchmark/BenchmarkArguments.swift
@@ -78,8 +78,9 @@ public struct BenchmarkArguments: ParsableArguments {
         if isDebug && !allowDebugBuild {
             throw ValidationError(debugBuildErrorMessage)
         }
-        if iterations != nil && iterations! <= 0 {
-            throw ValidationError(positiveNumberError(flag: "--iterations", of: "integer"))
+        if iterations != nil && iterations! < 0 {
+            throw ValidationError(
+                nonNegativeNumberError(flag: "--iterations", of: "integer"))
         }
         if warmupIterations != nil && warmupIterations! < 0 {
             throw ValidationError(
diff --git a/Sources/Benchmark/BenchmarkFilter.swift b/Sources/Benchmark/BenchmarkFilter.swift
index bf8b6d3..301f99d 100644
--- a/Sources/Benchmark/BenchmarkFilter.swift
+++ b/Sources/Benchmark/BenchmarkFilter.swift
@@ -31,7 +31,7 @@ internal struct BenchmarkFilter {
         if underlying == nil {
             return true
         } else {
-            let str = "\(suiteName)/\(benchmarkName)"
+            let str = "\(suiteName).\(benchmarkName)"
             let range = NSRange(location: 0, length: str.utf16.count)
             return underlying!.firstMatch(in: str, range: range) != nil
         }
diff --git a/Sources/Benchmark/BenchmarkReporter.swift b/Sources/Benchmark/BenchmarkReporter.swift
index 959090b..3283a25 100644
--- a/Sources/Benchmark/BenchmarkReporter.swift
+++ b/Sources/Benchmark/BenchmarkReporter.swift
@@ -89,7 +89,7 @@ struct PlainTextReporter<Target>: BenchmarkReporter where Target: TextOutputStre
         for result in results {
             let name: String
             if result.suiteName != "" {
-                name = "\(result.suiteName): \(result.benchmarkName)"
+                name = "\(result.suiteName).\(result.benchmarkName)"
             } else {
                 name = result.benchmarkName
             }


Test patch:
diff --git a/Tests/BenchmarkTests/BenchmarkCommandTests.swift b/Tests/BenchmarkTests/BenchmarkCommandTests.swift
index 121ee93..0a15b5f 100644
--- a/Tests/BenchmarkTests/BenchmarkCommandTests.swift
+++ b/Tests/BenchmarkTests/BenchmarkCommandTests.swift
@@ -47,7 +47,7 @@ final class BenchmarkCommandTests: XCTestCase {
             XCTAssert(filter.matches(suiteName: "foo", benchmarkName: "bar"))
         }
 
-        AssertParse(["--filter", "foo/bar", "--allow-debug-build"]) { settings in
+        AssertParse(["--filter", "foo.bar", "--allow-debug-build"]) { settings in
             let filter = try! BenchmarkFilter(settings.filter)
             XCTAssertFalse(filter.matches(suiteName: "foo", benchmarkName: "baz"))
             XCTAssertFalse(filter.matches(suiteName: "foobar", benchmarkName: "baz"))
@@ -61,10 +61,10 @@ final class BenchmarkCommandTests: XCTestCase {
         }
     }
 
-    func testParseZeroIterationsError() throws {
-        AssertFailsToParse(
-            ["--iterations", "0", "--allow-debug-build"],
-            with: "Value provided via --iterations must be a positive integer.")
+    func testParseZeroIterations() throws {
+        AssertParse(["--iterations", "0", "--allow-debug-build"]) { settings in
+            XCTAssertEqual(settings.iterations, 0)
+        }
     }
 
     func testParseWarmupIterations() throws {
@@ -114,7 +114,7 @@ final class BenchmarkCommandTests: XCTestCase {
         ("testDebugBuildError", testDebugBuildError),
         ("testParseFilter", testParseFilter),
         ("testParseIterations", testParseIterations),
-        ("testParseZeroIterationsError", testParseZeroIterationsError),
+        ("testParseZeroIterations", testParseZeroIterations),
         ("testParseWarmupIterations", testParseWarmupIterations),
         ("testParseZeroWarmupIterations", testParseZeroWarmupIterations),
         ("testParseNoWarmupIterations", testParseNoWarmupIterations),
diff --git a/Tests/BenchmarkTests/BenchmarkReporterTests.swift b/Tests/BenchmarkTests/BenchmarkReporterTests.swift
index 01aa5e6..80b24ae 100644
--- a/Tests/BenchmarkTests/BenchmarkReporterTests.swift
+++ b/Tests/BenchmarkTests/BenchmarkReporterTests.swift
@@ -34,23 +34,23 @@ final class BenchmarkReporterTests: XCTestCase {
     func testPlainTextReporter() throws {
         let results: [BenchmarkResult] = [
             BenchmarkResult(
-                benchmarkName: "fast", suiteName: "My Suite",
+                benchmarkName: "fast", suiteName: "MySuite",
                 settings: BenchmarkSettings(),
                 measurements: [1_000, 2_000],
                 warmupMeasurements: [],
                 counters: [:]),
             BenchmarkResult(
-                benchmarkName: "slow", suiteName: "My Suite",
+                benchmarkName: "slow", suiteName: "MySuite",
                 settings: BenchmarkSettings(),
                 measurements: [1_000_000, 2_000_000],
                 warmupMeasurements: [],
                 counters: [:]),
         ]
         let expected = #"""
-            name           time         std        iterations
-            -------------------------------------------------
-            My Suite: fast    1500.0 ns ±  47.14 %          2
-            My Suite: slow 1500000.0 ns ±  47.14 %          2
+            name         time         std        iterations
+            -----------------------------------------------
+            MySuite.fast    1500.0 ns ±  47.14 %          2
+            MySuite.slow 1500000.0 ns ±  47.14 %          2
             """#
         assertIsPrintedAs(results, expected)
     }
@@ -58,23 +58,23 @@ final class BenchmarkReporterTests: XCTestCase {
     func testCountersAreReported() throws {
         let results: [BenchmarkResult] = [
             BenchmarkResult(
-                benchmarkName: "fast", suiteName: "My Suite",
+                benchmarkName: "fast", suiteName: "MySuite",
                 settings: BenchmarkSettings(),
                 measurements: [1_000, 2_000],
                 warmupMeasurements: [],
-                counters: ["n": 7]),
+                counters: ["foo": 7]),
             BenchmarkResult(
-                benchmarkName: "slow", suiteName: "My Suite",
+                benchmarkName: "slow", suiteName: "MySuite",
                 settings: BenchmarkSettings(),
                 measurements: [1_000_000, 2_000_000],
                 warmupMeasurements: [],
                 counters: [:]),
         ]
         let expected = #"""
-            name           time         std        iterations n
-            -----------------------------------------------------
-            My Suite: fast    1500.0 ns ±  47.14 %          2 7.0
-            My Suite: slow 1500000.0 ns ±  47.14 %          2
+            name         time         std        iterations foo
+            ---------------------------------------------------
+            MySuite.fast    1500.0 ns ±  47.14 %          2 7.0
+            MySuite.slow 1500000.0 ns ±  47.14 %          2
             """#
         assertIsPrintedAs(results, expected)
     }
@@ -82,23 +82,23 @@ final class BenchmarkReporterTests: XCTestCase {
     func testWarmupReported() throws {
         let results: [BenchmarkResult] = [
             BenchmarkResult(
-                benchmarkName: "fast", suiteName: "My Suite",
+                benchmarkName: "fast", suiteName: "MySuite",
                 settings: BenchmarkSettings(),
                 measurements: [1_000, 2_000],
                 warmupMeasurements: [10, 20, 30],
                 counters: [:]),
             BenchmarkResult(
-                benchmarkName: "slow", suiteName: "My Suite",
+                benchmarkName: "slow", suiteName: "MySuite",
                 settings: BenchmarkSettings(),
                 measurements: [1_000_000, 2_000_000],
                 warmupMeasurements: [],
                 counters: [:]),
         ]
         let expected = #"""
-            name           time         std        iterations warmup
-            ---------------------------------------------------------
-            My Suite: fast    1500.0 ns ±  47.14 %          2 60.0 ns
-            My Suite: slow 1500000.0 ns ±  47.14 %          2
+            name         time         std        iterations warmup
+            -------------------------------------------------------
+            MySuite.fast    1500.0 ns ±  47.14 %          2 60.0 ns
+            MySuite.slow 1500000.0 ns ±  47.14 %          2
             """#
         assertIsPrintedAs(results, expected)
     }
@@ -106,37 +106,37 @@ final class BenchmarkReporterTests: XCTestCase {
     func testTimeUnitReported() throws {
         let results: [BenchmarkResult] = [
             BenchmarkResult(
-                benchmarkName: "ns", suiteName: "My Suite",
+                benchmarkName: "ns", suiteName: "MySuite",
                 settings: BenchmarkSettings([TimeUnit(.ns)]),
                 measurements: [123_456_789],
                 warmupMeasurements: [],
                 counters: [:]),
             BenchmarkResult(
-                benchmarkName: "us", suiteName: "My Suite",
+                benchmarkName: "us", suiteName: "MySuite",
                 settings: BenchmarkSettings([TimeUnit(.us)]),
                 measurements: [123_456_789],
                 warmupMeasurements: [],
                 counters: [:]),
             BenchmarkResult(
-                benchmarkName: "ms", suiteName: "My Suite",
+                benchmarkName: "ms", suiteName: "MySuite",
                 settings: BenchmarkSettings([TimeUnit(.ms)]),
                 measurements: [123_456_789],
                 warmupMeasurements: [],
                 counters: [:]),
             BenchmarkResult(
-                benchmarkName: "s", suiteName: "My Suite",
+                benchmarkName: "s", suiteName: "MySuite",
                 settings: BenchmarkSettings([TimeUnit(.s)]),
                 measurements: [123_456_789],
                 warmupMeasurements: [],
                 counters: [:]),
         ]
         let expected = #"""
-            name         time           std        iterations
-            -------------------------------------------------
-            My Suite: ns 123456789.0 ns ±   0.00 %          1
-            My Suite: us  123456.789 us ±   0.00 %          1
-            My Suite: ms  1234.56789 ms ±   0.00 %          1
-            My Suite: s   0.123456789 s ±   0.00 %          1
+            name       time           std        iterations
+            -----------------------------------------------
+            MySuite.ns 123456789.0 ns ±   0.00 %          1
+            MySuite.us  123456.789 us ±   0.00 %          1
+            MySuite.ms  1234.56789 ms ±   0.00 %          1
+            MySuite.s   0.123456789 s ±   0.00 %          1
             """#
         assertIsPrintedAs(results, expected)
     }
diff --git a/Tests/BenchmarkTests/BenchmarkRunnerTests.swift b/Tests/BenchmarkTests/BenchmarkRunnerTests.swift
index 5c3e505..230ddba 100644
--- a/Tests/BenchmarkTests/BenchmarkRunnerTests.swift
+++ b/Tests/BenchmarkTests/BenchmarkRunnerTests.swift
@@ -19,23 +19,23 @@ import XCTest
 final class BenchmarkRunnerTests: XCTestCase {
     func testFilterBenchmarksSuffix() throws {
         let settings: [BenchmarkSetting] = [Iterations(1), Filter("b1")]
-        XCTAssertEqual(Set(["suite1/b1", "suite2/b1"]), runBenchmarks(settings: settings))
+        XCTAssertEqual(Set(["suite1.b1", "suite2.b1"]), runBenchmarks(settings: settings))
     }
 
     func testFilterBenchmarksSuiteName() throws {
         let settings: [BenchmarkSetting] = [Iterations(1), Filter("suite1")]
-        XCTAssertEqual(Set(["suite1/b1", "suite1/b2"]), runBenchmarks(settings: settings))
+        XCTAssertEqual(Set(["suite1.b1", "suite1.b2"]), runBenchmarks(settings: settings))
     }
 
     func testFilterBenchmarksFullName() throws {
-        let settings: [BenchmarkSetting] = [Iterations(1), Filter("suite1/b1")]
-        XCTAssertEqual(Set(["suite1/b1"]), runBenchmarks(settings: settings))
+        let settings: [BenchmarkSetting] = [Iterations(1), Filter("suite1.b1")]
+        XCTAssertEqual(Set(["suite1.b1"]), runBenchmarks(settings: settings))
     }
 
     func testAutomaticallyDetectIterations() throws {
         let settings: [BenchmarkSetting] = []
         XCTAssertEqual(
-            Set(["suite2/b2", "suite2/b1", "suite1/b2", "suite1/b1"]),
+            Set(["suite2.b2", "suite2.b1", "suite1.b2", "suite1.b1"]),
             runBenchmarks(settings: settings))
     }
 
@@ -140,10 +140,10 @@ extension BenchmarkRunnerTests {
 
         var benchmarksRun = Set<String>()
 
-        suite1.benchmark("b1") { benchmarksRun.insert("suite1/b1") }
-        suite2.benchmark("b1") { benchmarksRun.insert("suite2/b1") }
-        suite1.benchmark("b2") { benchmarksRun.insert("suite1/b2") }
-        suite2.benchmark("b2") { benchmarksRun.insert("suite2/b2") }
+        suite1.benchmark("b1") { benchmarksRun.insert("suite1.b1") }
+        suite2.benchmark("b1") { benchmarksRun.insert("suite2.b1") }
+        suite1.benchmark("b2") { benchmarksRun.insert("suite1.b2") }
+        suite2.benchmark("b2") { benchmarksRun.insert("suite2.b2") }
 
         do {
             let _ = try run(suites: [suite1, suite2], settings: settings)