fix: Fix absolute comparisons (#123)

Fix incorrect scaling of delta absolute comparisons, add one more unit test.
ordo-one · Mar 22, 2023 · 6c80fcf · 6c80fcf
1 parent b558e61
commit 6c80fcf
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 7 deletions.
diff --git a/Sources/BenchmarkSupport/BenchmarkResult.swift b/Sources/BenchmarkSupport/BenchmarkResult.swift
@@ -379,7 +379,6 @@ public struct BenchmarkResult: Codable, Comparable, Equatable {
             let reverseComparison = metric.polarity == .prefersLarger
             var violationDescriptions: [ThresholdDeviation] = []
             var thresholdViolated = false
-
             if let threshold = thresholds.relative[percentile] {
                 if reverseComparison ? relativeDifference > threshold : -relativeDifference > threshold {
                     let relativeDiff = Statistics.roundToDecimalplaces(abs(relativeDifference), 1)
@@ -397,8 +396,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable {
                 }
             }
 
-            if var threshold = thresholds.absolute[percentile] {
-                threshold /= (1_000_000_000 / scalingFactor.rawValue)
+            if let threshold = thresholds.absolute[percentile] {
                 if reverseComparison ? -absoluteDifference > threshold : absoluteDifference > threshold {
                     violationDescriptions.append(ThresholdDeviation(name: name,
                                                                     target: target,
@@ -407,7 +405,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable {
                                                                     baseValue: normalize(lhs),
                                                                     comparisonValue: normalize(rhs),
                                                                     difference: normalize(absoluteDifference),
-                                                                    differenceThreshold: threshold,
+                                                                    differenceThreshold: normalize(threshold),
                                                                     relative: false,
                                                                     units: scalingFactor))
                     thresholdViolated = true

diff --git a/Tests/BenchmarkTests/BenchmarkResultTests.swift b/Tests/BenchmarkTests/BenchmarkResultTests.swift
@@ -173,13 +173,13 @@ final class BenchmarkResultTests: XCTestCase {
         firstStatistics.add(200)
 
         let secondStatistics = Statistics()
-        secondStatistics.add(0)
-        secondStatistics.add(126)
+        secondStatistics.add(5)
+        secondStatistics.add(136)
         secondStatistics.add(160)
         secondStatistics.add(175)
         secondStatistics.add(190)
         secondStatistics.add(199)
-        secondStatistics.add(200)
+        secondStatistics.add(210)
 
         let relative: BenchmarkThresholds.RelativeThresholds = [.p0: 0.0,
                                                                 .p25: 0.0,
@@ -249,6 +249,88 @@ final class BenchmarkResultTests: XCTestCase {
         XCTAssert(betterOrEqual)
     }
 
+    func testBenchmarkAbsoluteThresholds() throws {
+        let firstStatistics = Statistics()
+        firstStatistics.add(0)
+        firstStatistics.add(125)
+        firstStatistics.add(150)
+        firstStatistics.add(175)
+        firstStatistics.add(190)
+        firstStatistics.add(199)
+        firstStatistics.add(200)
+
+        let secondStatistics = Statistics()
+        secondStatistics.add(5)
+        secondStatistics.add(136)
+        secondStatistics.add(160)
+        secondStatistics.add(175)
+        secondStatistics.add(190)
+        secondStatistics.add(199)
+        secondStatistics.add(210)
+
+        let thirdStatistics = Statistics()
+        thirdStatistics.add(1_501)
+        thirdStatistics.add(1_501)
+        thirdStatistics.add(1_501)
+        thirdStatistics.add(1_501)
+        thirdStatistics.add(1_501)
+
+        let absolute: BenchmarkThresholds.AbsoluteThresholds = [.p0: 1,
+                                                                .p25: 1,
+                                                                .p50: 1,
+                                                                .p75: 1,
+                                                                .p90: 1,
+                                                                .p99: 1]
+
+        let absoluteThresholds = BenchmarkThresholds(absolute: absolute)
+
+        let absoluteTwo: BenchmarkThresholds.AbsoluteThresholds = [.p0: 1_500,
+                                                                   .p25: 1_500,
+                                                                   .p50: 1_500,
+                                                                   .p75: 1_500,
+                                                                   .p90: 1_500,
+                                                                   .p99: 1_500]
+
+        let absoluteThresholdsTwo = BenchmarkThresholds(absolute: absoluteTwo)
+
+        let firstResult = BenchmarkResult(metric: .cpuUser,
+                                          timeUnits: .nanoseconds,
+                                          scalingFactor: .one,
+                                          warmupIterations: 0,
+                                          thresholds: .default,
+                                          statistics: firstStatistics)
+
+        let secondResult = BenchmarkResult(metric: .cpuUser,
+                                           timeUnits: .nanoseconds,
+                                           scalingFactor: .one,
+                                           warmupIterations: 0,
+                                           thresholds: .default,
+                                           statistics: secondStatistics)
+
+        let thirdResult = BenchmarkResult(metric: .cpuUser,
+                                          timeUnits: .nanoseconds,
+                                          scalingFactor: .one,
+                                          warmupIterations: 0,
+                                          thresholds: .default,
+                                          statistics: thirdStatistics)
+
+        var (betterOrEqual, failures) = secondResult.betterResultsOrEqual(than: firstResult,
+                                                                          thresholds: absoluteThresholds)
+        XCTAssertFalse(betterOrEqual)
+        XCTAssertFalse(failures.isEmpty, "Failures: \(failures)")
+
+        (betterOrEqual, failures) = firstResult.betterResultsOrEqual(than: secondResult,
+                                                                     thresholds: absoluteThresholds)
+        XCTAssert(betterOrEqual)
+        XCTAssert(failures.isEmpty)
+
+        Benchmark.checkAbsoluteThresholds = true
+        failures = thirdResult.failsAbsoluteThresholdChecks(thresholds: absoluteThresholdsTwo,
+                                                            name: "test",
+                                                            target: "test")
+        XCTAssert(failures.count > 4)
+    }
+
     func testBenchmarkResultDescriptions() throws {
         let firstStatistics = Statistics()
         firstStatistics.add(0)