Repository: pion/interceptor
Pull request #194 (2023-06-26 17:07:19)

Enable injecting ticker to Sender Report interceptor
#### Description

This lets consumers use the Sender Report interceptor in tests that utilize fake clocks. Previously the current time could be injected, but the interceptor would only run according to real-time based on `time.NewTicker`, but now tests can control when the reports are generated.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pion/interceptor/pull/194?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
Patch coverage: **`100.00`**% and project coverage change: **`+0.18`** :tada:
> Comparison is base [(`aa50907`)](https://app.codecov.io/gh/pion/interceptor/commit/aa5090725bac04ef3612404cf03ff7f27a8d6bdf?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) 79.23% compared to head [(`1c1b3fb`)](https://app.codecov.io/gh/pion/interceptor/pull/194?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) 79.41%.

> :exclamation: Current head 1c1b3fb differs from pull request most recent head fd997ba. Consider uploading reports for the commit fd997ba to get more accurate results

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #194      +/-   ##
==========================================
+ Coverage   79.23%   79.41%   +0.18%     
==========================================
  Files          66       67       +1     
  Lines        3351     3367      +16     
==========================================
+ Hits         2655     2674      +19     
+ Misses        576      574       -2     
+ Partials      120      119       -1     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `79.41% <100.00%> (+0.18%)` | :arrow_up: |
| wasm | `77.19% <100.00%> (+0.10%)` | :arrow_up: |

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://app.codecov.io/gh/pion/interceptor/pull/194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [pkg/report/sender\_interceptor.go](https://app.codecov.io/gh/pion/interceptor/pull/194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL3JlcG9ydC9zZW5kZXJfaW50ZXJjZXB0b3IuZ28=) | `82.19% <100.00%> (+1.03%)` | :arrow_up: |
| [pkg/report/sender\_option.go](https://app.codecov.io/gh/pion/interceptor/pull/194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL3JlcG9ydC9zZW5kZXJfb3B0aW9uLmdv) | `100.00% <100.00%> (ø)` | |
| [pkg/report/ticker.go](https://app.codecov.io/gh/pion/interceptor/pull/194?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL3JlcG9ydC90aWNrZXIuZ28=) | `100.00% <100.00%> (ø)` | |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/pion/interceptor/pull/194/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/pion/interceptor/pull/194?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).


Patch:
diff --git a/pkg/report/sender_interceptor.go b/pkg/report/sender_interceptor.go
index 02d1c00..508d7e9 100644
--- a/pkg/report/sender_interceptor.go
+++ b/pkg/report/sender_interceptor.go
@@ -13,6 +13,9 @@ import (
 	"github.com/pion/rtp"
 )
 
+// TickerFactory is a factory to create new tickers
+type TickerFactory func(d time.Duration) Ticker
+
 // SenderInterceptorFactory is a interceptor.Factory for a SenderInterceptor
 type SenderInterceptorFactory struct {
 	opts []SenderOption
@@ -23,8 +26,11 @@ func (s *SenderInterceptorFactory) NewInterceptor(_ string) (interceptor.Interce
 	i := &SenderInterceptor{
 		interval: 1 * time.Second,
 		now:      time.Now,
-		log:      logging.NewDefaultLoggerFactory().NewLogger("sender_interceptor"),
-		close:    make(chan struct{}),
+		newTicker: func(d time.Duration) Ticker {
+			return &timeTicker{time.NewTicker(d)}
+		},
+		log:   logging.NewDefaultLoggerFactory().NewLogger("sender_interceptor"),
+		close: make(chan struct{}),
 	}
 
 	for _, opt := range s.opts {
@@ -44,13 +50,15 @@ func NewSenderInterceptor(opts ...SenderOption) (*SenderInterceptorFactory, erro
 // SenderInterceptor interceptor generates sender reports.
 type SenderInterceptor struct {
 	interceptor.NoOp
-	interval time.Duration
-	now      func() time.Time
-	streams  sync.Map
-	log      logging.LeveledLogger
-	m        sync.Mutex
-	wg       sync.WaitGroup
-	close    chan struct{}
+	interval  time.Duration
+	now       func() time.Time
+	newTicker TickerFactory
+	streams   sync.Map
+	log       logging.LeveledLogger
+	m         sync.Mutex
+	wg        sync.WaitGroup
+	close     chan struct{}
+	started   chan struct{}
 }
 
 func (s *SenderInterceptor) isClosed() bool {
@@ -95,11 +103,16 @@ func (s *SenderInterceptor) BindRTCPWriter(writer interceptor.RTCPWriter) interc
 func (s *SenderInterceptor) loop(rtcpWriter interceptor.RTCPWriter) {
 	defer s.wg.Done()
 
-	ticker := time.NewTicker(s.interval)
+	ticker := s.newTicker(s.interval)
 	defer ticker.Stop()
+	if s.started != nil {
+		// This lets us synchronize in tests to know whether the loop has begun or not.
+		// It only happens if started was initialized, which should not occur in non-tests.
+		close(s.started)
+	}
 	for {
 		select {
-		case <-ticker.C:
+		case <-ticker.Ch():
 			now := s.now()
 			s.streams.Range(func(key, value interface{}) bool {
 				if stream, ok := value.(*senderStream); !ok {
diff --git a/pkg/report/sender_option.go b/pkg/report/sender_option.go
index 6932e1c..50abafb 100644
--- a/pkg/report/sender_option.go
+++ b/pkg/report/sender_option.go
@@ -35,3 +35,20 @@ func SenderNow(f func() time.Time) SenderOption {
 		return nil
 	}
 }
+
+// SenderTicker sets an alternative for the time.NewTicker function.
+func SenderTicker(f TickerFactory) SenderOption {
+	return func(r *SenderInterceptor) error {
+		r.newTicker = f
+		return nil
+	}
+}
+
+// enableStartTracking is used by tests to synchronize whether the loop() has begun
+// and it's safe to start sending ticks to the ticker.
+func enableStartTracking(startedCh chan struct{}) SenderOption {
+	return func(r *SenderInterceptor) error {
+		r.started = startedCh
+		return nil
+	}
+}
diff --git a/pkg/report/ticker.go b/pkg/report/ticker.go
new file mode 100644
index 0000000..59a8c6c
--- /dev/null
+++ b/pkg/report/ticker.go
@@ -0,0 +1,20 @@
+// SPDX-FileCopyrightText: 2023 The Pion community <https://pion.ly>
+// SPDX-License-Identifier: MIT
+
+package report
+
+import "time"
+
+// Ticker is an interface for *time.Ticker for use with the SenderTicker option.
+type Ticker interface {
+	Ch() <-chan time.Time
+	Stop()
+}
+
+type timeTicker struct {
+	*time.Ticker
+}
+
+func (t *timeTicker) Ch() <-chan time.Time {
+	return t.C
+}


Test patch:
diff --git a/pkg/report/sender_interceptor_test.go b/pkg/report/sender_interceptor_test.go
index 5647f8e..cc034fb 100644
--- a/pkg/report/sender_interceptor_test.go
+++ b/pkg/report/sender_interceptor_test.go
@@ -91,4 +91,42 @@ func TestSenderInterceptor(t *testing.T) {
 			OctetCount:  20,
 		}, sr)
 	})
+
+	t.Run("inject ticker", func(t *testing.T) {
+		mNow := &test.MockTime{}
+		mTick := &test.MockTicker{
+			C: make(chan time.Time),
+		}
+		advanceTicker := func() {
+			mNow.SetNow(mNow.Now().Add(50 * time.Millisecond))
+			mTick.Tick(mNow.Now())
+		}
+		loopStarted := make(chan struct{})
+		f, err := NewSenderInterceptor(
+			SenderInterval(time.Millisecond*50),
+			SenderLog(logging.NewDefaultLoggerFactory().NewLogger("test")),
+			SenderNow(mNow.Now),
+			SenderTicker(func(d time.Duration) Ticker { return mTick }),
+			enableStartTracking(loopStarted),
+		)
+		assert.NoError(t, err)
+
+		i, err := f.NewInterceptor("")
+		assert.NoError(t, err)
+
+		stream := test.NewMockStream(&interceptor.StreamInfo{
+			SSRC:      123456,
+			ClockRate: 90000,
+		}, i)
+		defer func() {
+			assert.NoError(t, stream.Close())
+		}()
+
+		<-loopStarted
+		for i := 0; i < 5; i++ {
+			advanceTicker()
+			pkts := <-stream.WrittenRTCP()
+			assert.Equal(t, len(pkts), 1)
+		}
+	})
 }