Repository: pion/interceptor
Pull request #105 (2022-03-08 09:40:20)

Fix and test clamping functions
This PR fixes a bug in `clampInt` and tests `clampInt` and `clampDuration`. Without this fix, `clampInt(a, a, b)` results in `b`, but it should be `a`. `clampDuration` was correct before, but now uses the same logic as `clampInt` and both are tested.

codecov[bot]: # [Codecov](https://codecov.io/gh/pion/interceptor/pull/105?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
> Merging [#105](https://codecov.io/gh/pion/interceptor/pull/105?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (143bac6) into [master](https://codecov.io/gh/pion/interceptor/commit/ca45006d6254c9b41de889715838e54c15217b96?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (ca45006) will **increase** coverage by `0.14%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/interceptor/pull/105/graphs/tree.svg?width=650&height=150&src=pr&token=FNH9xUG6AX&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)](https://codecov.io/gh/pion/interceptor/pull/105?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)

```diff
@@            Coverage Diff             @@
##           master     #105      +/-   ##
==========================================
+ Coverage   79.17%   79.31%   +0.14%     
==========================================
  Files          51       51              
  Lines        2468     2461       -7     
==========================================
- Hits         1954     1952       -2     
+ Misses        424      419       -5     
  Partials       90       90              
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `79.31% <100.00%> (+0.34%)` | :arrow_up: |
| wasm | `?` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/interceptor/pull/105?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [pkg/gcc/gcc.go](https://codecov.io/gh/pion/interceptor/pull/105/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL2djYy9nY2MuZ28=) | `100.00% <100.00%> (+23.80%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/interceptor/pull/105?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/interceptor/pull/105?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Last update [ca45006...143bac6](https://codecov.io/gh/pion/interceptor/pull/105?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).


Patch:
diff --git a/pkg/gcc/gcc.go b/pkg/gcc/gcc.go
index a857ebf..3961eb6 100644
--- a/pkg/gcc/gcc.go
+++ b/pkg/gcc/gcc.go
@@ -10,22 +10,17 @@ func minInt(a, b int) int {
 	return b
 }
 
-func clampInt(b, min, max int) int {
-	if min < b && b < max {
-		return b
-	}
-	if b < min {
-		return min
+func maxInt(a, b int) int {
+	if a > b {
+		return a
 	}
-	return max
+	return b
+}
+
+func clampInt(b, min, max int) int {
+	return maxInt(min, minInt(max, b))
 }
 
 func clampDuration(d, min, max time.Duration) time.Duration {
-	if min <= d && d <= max {
-		return d
-	}
-	if d <= min {
-		return min
-	}
-	return max
+	return time.Duration(clampInt(int(d), int(min), int(max)))
 }


Test patch:
diff --git a/pkg/gcc/gcc_test.go b/pkg/gcc/gcc_test.go
new file mode 100644
index 0000000..47f8adf
--- /dev/null
+++ b/pkg/gcc/gcc_test.go
@@ -0,0 +1,120 @@
+package gcc
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMinInt(t *testing.T) {
+	tests := []struct {
+		expected int
+		a, b     int
+	}{
+		{
+			expected: 0,
+			a:        0,
+			b:        100,
+		},
+		{
+			expected: 10,
+			a:        10,
+			b:        10,
+		},
+		{
+			expected: 1,
+			a:        10,
+			b:        1,
+		},
+	}
+	for i, tt := range tests {
+		tt := tt
+		t.Run(fmt.Sprintf("%v", i), func(t *testing.T) {
+			assert.Equal(t, tt.expected, minInt(tt.a, tt.b))
+		})
+	}
+}
+
+func TestMaxInt(t *testing.T) {
+	tests := []struct {
+		expected int
+		a, b     int
+	}{
+		{
+			expected: 100,
+			a:        0,
+			b:        100,
+		},
+		{
+			expected: 10,
+			a:        10,
+			b:        10,
+		},
+		{
+			expected: 10,
+			a:        10,
+			b:        1,
+		},
+	}
+	for i, tt := range tests {
+		tt := tt
+		t.Run(fmt.Sprintf("%v", i), func(t *testing.T) {
+			assert.Equal(t, tt.expected, maxInt(tt.a, tt.b))
+		})
+	}
+}
+
+func TestClamp(t *testing.T) {
+	tests := []struct {
+		expected int
+		x        int
+		min      int
+		max      int
+	}{
+		{
+			expected: 50,
+			x:        50,
+			min:      0,
+			max:      100,
+		},
+		{
+			expected: 50,
+			x:        50,
+			min:      50,
+			max:      100,
+		},
+		{
+			expected: 100,
+			x:        100,
+			min:      0,
+			max:      100,
+		},
+		{
+			expected: 50,
+			x:        3,
+			min:      50,
+			max:      100,
+		},
+		{
+			expected: 100,
+			x:        150,
+			min:      0,
+			max:      100,
+		},
+	}
+	for i, tt := range tests {
+		tt := tt
+		t.Run(fmt.Sprintf("int/%v", i), func(t *testing.T) {
+			assert.Equal(t, tt.expected, clampInt(tt.x, tt.min, tt.max))
+		})
+		t.Run(fmt.Sprintf("duration/%v", i), func(t *testing.T) {
+			x := time.Duration(tt.x)
+			min := time.Duration(tt.min)
+			max := time.Duration(tt.max)
+			expected := time.Duration(tt.expected)
+			assert.Equal(t, expected, clampDuration(x, min, max))
+		})
+	}
+}