From 7024f36a76e3557bb672fc9264a306a7314d971b Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Mon, 25 Nov 2019 11:46:29 +0100 Subject: [PATCH] general: adding the Clock interface to aid testing This commit adds Clock and DefaultClock and moves the private invoices.testClock under the clock package while adding basic unit tests for it. Clock is an interface currently encapsulating Now() and TickAfter(). It can be added as an external dependency to any class. This way tests can stub out time.Now() or time.After(). The DefaultClock class simply returns the real time.Now() and time.After(). --- clock/default_clock.go | 24 +++++++ clock/interface.go | 16 +++++ invoices/clock_test.go => clock/test_clock.go | 28 ++++----- clock/test_clock_test.go | 63 +++++++++++++++++++ invoices/invoiceregistry.go | 15 +++-- invoices/invoiceregistry_test.go | 2 +- invoices/test_utils_test.go | 10 +-- server.go | 4 +- 8 files changed, 131 insertions(+), 31 deletions(-) create mode 100644 clock/default_clock.go create mode 100644 clock/interface.go rename invoices/clock_test.go => clock/test_clock.go (65%) create mode 100644 clock/test_clock_test.go diff --git a/clock/default_clock.go b/clock/default_clock.go new file mode 100644 index 00000000..3a4f8df3 --- /dev/null +++ b/clock/default_clock.go @@ -0,0 +1,24 @@ +package clock + +import ( + "time" +) + +// DefaultClock implements Clock interface by simply calling the appropriate +// time functions. +type DefaultClock struct{} + +// NewDefaultClock constructs a new DefaultClock. +func NewDefaultClock() Clock { + return &DefaultClock{} +} + +// Now simply returns time.Now(). +func (DefaultClock) Now() time.Time { + return time.Now() +} + +// TickAfter simply wraps time.After(). +func (DefaultClock) TickAfter(duration time.Duration) <-chan time.Time { + return time.After(duration) +} diff --git a/clock/interface.go b/clock/interface.go new file mode 100644 index 00000000..0450410e --- /dev/null +++ b/clock/interface.go @@ -0,0 +1,16 @@ +package clock + +import ( + "time" +) + +// Clock is an interface that provides a time functions for LND packages. +// This is useful during testing when a concrete time reference is needed. +type Clock interface { + // Now returns the current local time (as defined by the Clock). + Now() time.Time + + // TickAfter returns a channel that will receive a tick after the specified + // duration has passed. + TickAfter(duration time.Duration) <-chan time.Time +} diff --git a/invoices/clock_test.go b/clock/test_clock.go similarity index 65% rename from invoices/clock_test.go rename to clock/test_clock.go index 41dd4991..f4319cee 100644 --- a/invoices/clock_test.go +++ b/clock/test_clock.go @@ -1,42 +1,40 @@ -package invoices +package clock import ( "sync" "time" ) -// testClock can be used in tests to mock time. -type testClock struct { +// TestClock can be used in tests to mock time. +type TestClock struct { currentTime time.Time timeChanMap map[time.Time][]chan time.Time timeLock sync.Mutex } -// newTestClock returns a new test clock. -func newTestClock(startTime time.Time) *testClock { - return &testClock{ +// NewTestClock returns a new test clock. +func NewTestClock(startTime time.Time) *TestClock { + return &TestClock{ currentTime: startTime, timeChanMap: make(map[time.Time][]chan time.Time), } } -// now returns the current (test) time. -func (c *testClock) now() time.Time { +// Now returns the current (test) time. +func (c *TestClock) Now() time.Time { c.timeLock.Lock() defer c.timeLock.Unlock() return c.currentTime } -// tickAfter returns a channel that will receive a tick at the specified time. -func (c *testClock) tickAfter(duration time.Duration) <-chan time.Time { +// TickAfter returns a channel that will receive a tick after the specified +// duration has passed passed by the user set test time. +func (c *TestClock) TickAfter(duration time.Duration) <-chan time.Time { c.timeLock.Lock() defer c.timeLock.Unlock() triggerTime := c.currentTime.Add(duration) - log.Debugf("tickAfter called: duration=%v, trigger_time=%v", - duration, triggerTime) - ch := make(chan time.Time, 1) // If already expired, tick immediately. @@ -53,8 +51,8 @@ func (c *testClock) tickAfter(duration time.Duration) <-chan time.Time { return ch } -// setTime sets the (test) time and triggers tick channels when they expire. -func (c *testClock) setTime(now time.Time) { +// SetTime sets the (test) time and triggers tick channels when they expire. +func (c *TestClock) SetTime(now time.Time) { c.timeLock.Lock() defer c.timeLock.Unlock() diff --git a/clock/test_clock_test.go b/clock/test_clock_test.go new file mode 100644 index 00000000..879cc8fd --- /dev/null +++ b/clock/test_clock_test.go @@ -0,0 +1,63 @@ +package clock + +import ( + "testing" + "time" +) + +var ( + testTime = time.Date(2009, time.January, 3, 12, 0, 0, 0, time.UTC) +) + +func TestNow(t *testing.T) { + c := NewTestClock(testTime) + now := c.Now() + + if now != testTime { + t.Fatalf("expected: %v, got: %v", testTime, now) + } + + now = now.Add(time.Hour) + c.SetTime(now) + if c.Now() != now { + t.Fatalf("epected: %v, got: %v", now, c.Now()) + } +} + +func TestTickAfter(t *testing.T) { + c := NewTestClock(testTime) + + // Should be ticking immediately. + ticker0 := c.TickAfter(0) + + // Both should be ticking after SetTime + ticker1 := c.TickAfter(time.Hour) + ticker2 := c.TickAfter(time.Hour) + + // We don't expect this one to tick. + ticker3 := c.TickAfter(2 * time.Hour) + + tickOrTimeOut := func(ticker <-chan time.Time, expectTick bool) { + tick := false + select { + case <-ticker: + tick = true + case <-time.After(time.Millisecond): + } + + if tick != expectTick { + t.Fatalf("expected tick: %v, ticked: %v", expectTick, tick) + } + } + + tickOrTimeOut(ticker0, true) + tickOrTimeOut(ticker1, false) + tickOrTimeOut(ticker2, false) + tickOrTimeOut(ticker3, false) + + c.SetTime(c.Now().Add(time.Hour)) + + tickOrTimeOut(ticker1, true) + tickOrTimeOut(ticker2, true) + tickOrTimeOut(ticker3, false) +} diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index bfdc54a6..16f4f400 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -9,6 +9,7 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/clock" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/queue" @@ -62,12 +63,10 @@ type RegistryConfig struct { // waiting for the other set members to arrive. HtlcHoldDuration time.Duration - // Now returns the current time. - Now func() time.Time - - // TickAfter returns a channel that is sent on after the specified - // duration as passed. - TickAfter func(duration time.Duration) <-chan time.Time + // Clock holds the clock implementation that is used to provide + // Now() and TickAfter() and is useful to stub out the clock functions + // during testing. + Clock clock.Clock } // htlcReleaseEvent describes an htlc auto-release event. It is used to release @@ -177,8 +176,8 @@ type invoiceEvent struct { // tickAt returns a channel that ticks at the specified time. If the time has // already passed, it will tick immediately. func (i *InvoiceRegistry) tickAt(t time.Time) <-chan time.Time { - now := i.cfg.Now() - return i.cfg.TickAfter(t.Sub(now)) + now := i.cfg.Clock.Now() + return i.cfg.Clock.TickAfter(t.Sub(now)) } // invoiceEventLoop is the dedicated goroutine responsible for accepting diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index 567d2747..fbd9a260 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -587,7 +587,7 @@ func TestSettleMpp(t *testing.T) { } // Simulate mpp timeout releasing htlc 1. - ctx.clock.setTime(testTime.Add(30 * time.Second)) + ctx.clock.SetTime(testTime.Add(30 * time.Second)) hodlEvent := (<-hodlChan1).(HodlEvent) if hodlEvent.Preimage != nil { diff --git a/invoices/test_utils_test.go b/invoices/test_utils_test.go index 08796584..52e6044f 100644 --- a/invoices/test_utils_test.go +++ b/invoices/test_utils_test.go @@ -12,6 +12,7 @@ import ( "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/chaincfg" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/clock" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/record" @@ -120,27 +121,26 @@ func newTestChannelDB() (*channeldb.DB, func(), error) { type testContext struct { registry *InvoiceRegistry - clock *testClock + clock *clock.TestClock cleanup func() t *testing.T } func newTestContext(t *testing.T) *testContext { - clock := newTestClock(testTime) + clock := clock.NewTestClock(testTime) cdb, cleanup, err := newTestChannelDB() if err != nil { t.Fatal(err) } - cdb.Now = clock.now + cdb.Now = clock.Now // Instantiate and start the invoice ctx.registry. cfg := RegistryConfig{ FinalCltvRejectDelta: testFinalCltvRejectDelta, HtlcHoldDuration: 30 * time.Second, - Now: clock.now, - TickAfter: clock.tickAfter, + Clock: clock, } registry := NewRegistry(cdb, &cfg) diff --git a/server.go b/server.go index 88dce9de..edf90e76 100644 --- a/server.go +++ b/server.go @@ -33,6 +33,7 @@ import ( "github.com/lightningnetwork/lnd/chanfitness" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/channelnotifier" + "github.com/lightningnetwork/lnd/clock" "github.com/lightningnetwork/lnd/contractcourt" "github.com/lightningnetwork/lnd/discovery" "github.com/lightningnetwork/lnd/feature" @@ -381,8 +382,7 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB, registryConfig := invoices.RegistryConfig{ FinalCltvRejectDelta: defaultFinalCltvRejectDelta, HtlcHoldDuration: invoices.DefaultHtlcHoldDuration, - Now: time.Now, - TickAfter: time.After, + Clock: clock.NewDefaultClock(), } s := &server{