ipn/ipnlocal: reduce line noise in tests

Use helpers and variadic functions to make the call sites a lot easier to read, since they occur a lot. Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
3 years ago · 221cc5f8f2
parent 45d3174c0d
commit 221cc5f8f2
1 changed files with 47 additions and 49 deletions
--- a/ipn/ipnlocal/state_test.go
+++ b/ipn/ipnlocal/state_test.go
@ -86,6 +86,7 @@ func (nt *notifyThrottler) drain(count int) []ipn.Notify {
 // in the controlclient.Client, so by controlling it, we can check that
 // the state machine works as expected.
 type mockControl struct {
+	tb         testing.TB
 	opts       controlclient.Options
 	logf       logger.Logf
 	statusFunc func(controlclient.Status)
@ -97,9 +98,9 @@ type mockControl struct {
 	machineKey  key.MachinePrivate
 }

-func newMockControl() *mockControl {
+func newMockControl(tb testing.TB) *mockControl {
 	return &mockControl{
-		calls:       []string{},
+		tb:          tb,
 		authBlocked: true,
 	}
 }
@ -157,15 +158,14 @@ func (cc *mockControl) called(s string) {
 	cc.calls = append(cc.calls, s)
 }

-// getCalls returns the list of functions that have been called since the
-// last time getCalls was run.
-func (cc *mockControl) getCalls() []string {
+// assertCalls fails the test if the list of functions that have been called since the
+// last time assertCall was run does not match want.
+func (cc *mockControl) assertCalls(want ...string) {
+	cc.tb.Helper()
 	cc.mu.Lock()
 	defer cc.mu.Unlock()
-
-	r := cc.calls
-	cc.calls = []string{}
-	return r
+	qt.Assert(cc.tb, cc.calls, qt.DeepEquals, want)
+	cc.calls = nil
 }

 // setAuthBlocked changes the return value of AuthCantContinue.
@ -286,7 +286,7 @@ func TestStateMachine(t *testing.T) {
 	}
 	t.Cleanup(e.Close)

-	cc := newMockControl()
+	cc := newMockControl(t)
 	b, err := NewLocalBackend(logf, "logid", store, e)
 	if err != nil {
 		t.Fatalf("NewLocalBackend: %v", err)
@ -321,7 +321,7 @@ func TestStateMachine(t *testing.T) {

 	// Check that it hasn't called us right away.
 	// The state machine should be idle until we call Start().
-	c.Assert(cc.getCalls(), qt.HasLen, 0)
+	cc.assertCalls()

 	// Start the state machine.
 	// Since !WantRunning by default, it'll create a controlclient,
@ -331,10 +331,10 @@ func TestStateMachine(t *testing.T) {
 	c.Assert(b.Start(ipn.Options{StateKey: ipn.GlobalDaemonStateKey}), qt.IsNil)
 	{
 		// BUG: strictly, it should pause, not unpause, here, since !WantRunning.
-		c.Assert([]string{"New", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("New", "unpause")

 		nn := notifies.drain(2)
-		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		cc.assertCalls()
 		c.Assert(nn[0].Prefs, qt.IsNotNil)
 		c.Assert(nn[1].State, qt.IsNotNil)
 		prefs := *nn[0].Prefs
@ -356,10 +356,10 @@ func TestStateMachine(t *testing.T) {
 	c.Assert(b.Start(ipn.Options{StateKey: ipn.GlobalDaemonStateKey}), qt.IsNil)
 	{
 		// BUG: strictly, it should pause, not unpause, here, since !WantRunning.
-		c.Assert([]string{"Shutdown", "unpause", "New", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("Shutdown", "unpause", "New", "unpause")

 		nn := notifies.drain(2)
-		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		cc.assertCalls()
 		c.Assert(nn[0].Prefs, qt.IsNotNil)
 		c.Assert(nn[1].State, qt.IsNotNil)
 		c.Assert(nn[0].Prefs.LoggedOut, qt.IsFalse)
@ -375,7 +375,7 @@ func TestStateMachine(t *testing.T) {
 	notifies.expect(0)
 	b.Login(nil)
 	{
-		c.Assert(cc.getCalls(), qt.DeepEquals, []string{"Login"})
+		cc.assertCalls("Login")
 		notifies.drain(0)
 		// Note: WantRunning isn't true yet. It'll switch to true
 		// after a successful login finishes.
@ -391,7 +391,7 @@ func TestStateMachine(t *testing.T) {
 	url1 := "http://localhost:1/1"
 	cc.send(nil, url1, false, nil)
 	{
-		c.Assert(cc.getCalls(), qt.DeepEquals, []string{"unpause"})
+		cc.assertCalls("unpause")

 		// ...but backend eats that notification, because the user
 		// didn't explicitly request interactive login yet, and
@ -412,7 +412,7 @@ func TestStateMachine(t *testing.T) {
 	b.StartLoginInteractive()
 	{
 		nn := notifies.drain(1)
-		c.Assert([]string{"unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("unpause")
 		c.Assert(nn[0].BrowseToURL, qt.IsNotNil)
 		c.Assert(url1, qt.Equals, *nn[0].BrowseToURL)
 	}
@ -428,7 +428,7 @@ func TestStateMachine(t *testing.T) {
 	{
 		notifies.drain(0)
 		// backend asks control for another login sequence
-		c.Assert([]string{"Login"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("Login")
 	}

 	// Provide a new interactive login URL.
@ -437,7 +437,7 @@ func TestStateMachine(t *testing.T) {
 	url2 := "http://localhost:1/2"
 	cc.send(nil, url2, false, nil)
 	{
-		c.Assert([]string{"unpause", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("unpause", "unpause")

 		// This time, backend should emit it to the UI right away,
 		// because the UI is anxiously awaiting a new URL to visit.
@ -465,7 +465,7 @@ func TestStateMachine(t *testing.T) {
 		// wait until it gets into Starting.
 		// TODO: (Currently this test doesn't detect that bug, but
 		// it's visible in the logs)
-		c.Assert([]string{"unpause", "unpause", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("unpause", "unpause", "unpause")
 		c.Assert(nn[0].LoginFinished, qt.IsNotNil)
 		c.Assert(nn[1].Prefs, qt.IsNotNil)
 		c.Assert(nn[2].State, qt.IsNotNil)
@ -487,7 +487,7 @@ func TestStateMachine(t *testing.T) {
 	})
 	{
 		nn := notifies.drain(1)
-		c.Assert([]string{"unpause", "unpause", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("unpause", "unpause", "unpause")
 		c.Assert(nn[0].State, qt.IsNotNil)
 		c.Assert(ipn.Starting, qt.Equals, *nn[0].State)
 	}
@ -510,7 +510,7 @@ func TestStateMachine(t *testing.T) {
 	})
 	{
 		nn := notifies.drain(2)
-		c.Assert([]string{"pause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("pause")
 		// BUG: I would expect Prefs to change first, and state after.
 		c.Assert(nn[0].State, qt.IsNotNil)
 		c.Assert(nn[1].Prefs, qt.IsNotNil)
@ -528,7 +528,7 @@ func TestStateMachine(t *testing.T) {
 	{
 		nn := notifies.drain(2)
 		// BUG: Login isn't needed here. We never logged out.
-		c.Assert([]string{"Login", "unpause", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("Login", "unpause", "unpause")
 		// BUG: I would expect Prefs to change first, and state after.
 		c.Assert(nn[0].State, qt.IsNotNil)
 		c.Assert(nn[1].Prefs, qt.IsNotNil)
@ -548,7 +548,7 @@ func TestStateMachine(t *testing.T) {
 	c.Assert(b.Start(ipn.Options{StateKey: ipn.GlobalDaemonStateKey}), qt.IsNil)
 	{
 		nn := notifies.drain(1)
-		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		cc.assertCalls()
 		c.Assert(nn[0].State, qt.IsNotNil)
 		c.Assert(nn[0].LoginFinished, qt.IsNotNil)
 		c.Assert(nn[0].NetMap, qt.IsNotNil)
@ -565,7 +565,7 @@ func TestStateMachine(t *testing.T) {
 	b.Logout()
 	{
 		nn := notifies.drain(2)
-		c.Assert([]string{"pause", "StartLogout", "pause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("pause", "StartLogout", "pause")
 		c.Assert(nn[0].State, qt.IsNotNil)
 		c.Assert(nn[1].Prefs, qt.IsNotNil)
 		c.Assert(ipn.Stopped, qt.Equals, *nn[0].State)
@ -582,7 +582,7 @@ func TestStateMachine(t *testing.T) {
 	cc.send(nil, "", false, nil)
 	{
 		nn := notifies.drain(1)
-		c.Assert([]string{"unpause", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("unpause", "unpause")
 		c.Assert(nn[0].State, qt.IsNotNil)
 		c.Assert(ipn.NeedsLogin, qt.Equals, *nn[0].State)
 		c.Assert(b.Prefs().LoggedOut, qt.IsTrue)
@ -598,8 +598,8 @@ func TestStateMachine(t *testing.T) {
 		notifies.drain(0)
 		// BUG: the backend has already called StartLogout, and we're
 		// still logged out. So it shouldn't call it again.
-		c.Assert([]string{"StartLogout", "unpause"}, qt.DeepEquals, cc.getCalls())
-		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		cc.assertCalls("StartLogout", "unpause")
+		cc.assertCalls()
 		c.Assert(b.Prefs().LoggedOut, qt.IsTrue)
 		c.Assert(b.Prefs().WantRunning, qt.IsFalse)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
@ -612,7 +612,7 @@ func TestStateMachine(t *testing.T) {
 	cc.send(nil, "", false, nil)
 	{
 		notifies.drain(0)
-		c.Assert(cc.getCalls(), qt.DeepEquals, []string{"unpause", "unpause"})
+		cc.assertCalls("unpause", "unpause")
 		c.Assert(b.Prefs().LoggedOut, qt.IsTrue)
 		c.Assert(b.Prefs().WantRunning, qt.IsFalse)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
@ -626,8 +626,7 @@ func TestStateMachine(t *testing.T) {
 	// I guess, since that's supposed to be synchronous.
 	{
 		notifies.drain(0)
-		c.Assert([]string{"Logout", "unpause"}, qt.DeepEquals, cc.getCalls())
-		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		cc.assertCalls("Logout", "unpause")
 		c.Assert(b.Prefs().LoggedOut, qt.IsTrue)
 		c.Assert(b.Prefs().WantRunning, qt.IsFalse)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
@ -640,7 +639,7 @@ func TestStateMachine(t *testing.T) {
 	cc.send(nil, "", false, nil)
 	{
 		notifies.drain(0)
-		c.Assert(cc.getCalls(), qt.DeepEquals, []string{"unpause", "unpause"})
+		cc.assertCalls("unpause", "unpause")
 		c.Assert(b.Prefs().LoggedOut, qt.IsTrue)
 		c.Assert(b.Prefs().WantRunning, qt.IsFalse)
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
@ -653,7 +652,7 @@ func TestStateMachine(t *testing.T) {
 	{
 		notifies.drain(0)
 		// BUG: I expect a transition to ipn.NoState here.
-		c.Assert(cc.getCalls(), qt.DeepEquals, []string{"Shutdown"})
+		cc.assertCalls("Shutdown")
 	}

 	// Oh, you thought we were done? Ha! Now we have to test what
@ -670,10 +669,10 @@ func TestStateMachine(t *testing.T) {
 	{
 		// BUG: We already called Shutdown(), no need to do it again.
 		// BUG: don't unpause because we're not logged in.
-		c.Assert([]string{"Shutdown", "unpause", "New", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("Shutdown", "unpause", "New", "unpause")

 		nn := notifies.drain(2)
-		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		cc.assertCalls()
 		c.Assert(nn[0].Prefs, qt.IsNotNil)
 		c.Assert(nn[1].State, qt.IsNotNil)
 		c.Assert(nn[0].Prefs.LoggedOut, qt.IsTrue)
@ -695,7 +694,7 @@ func TestStateMachine(t *testing.T) {
 	})
 	{
 		nn := notifies.drain(3)
-		c.Assert([]string{"unpause", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("unpause", "unpause")
 		c.Assert(nn[0].LoginFinished, qt.IsNotNil)
 		c.Assert(nn[1].Prefs, qt.IsNotNil)
 		c.Assert(nn[2].State, qt.IsNotNil)
@ -715,7 +714,7 @@ func TestStateMachine(t *testing.T) {
 	})
 	{
 		nn := notifies.drain(2)
-		c.Assert([]string{"pause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("pause")
 		// BUG: I would expect Prefs to change first, and state after.
 		c.Assert(nn[0].State, qt.IsNotNil)
 		c.Assert(nn[1].Prefs, qt.IsNotNil)
@ -735,8 +734,7 @@ func TestStateMachine(t *testing.T) {
 		//  name, etc when in state ipn.Stopped.
 		//  Arguably they shouldn't try. But they currently do.
 		nn := notifies.drain(2)
-		c.Assert([]string{"Shutdown", "unpause", "New", "Login", "unpause"}, qt.DeepEquals, cc.getCalls())
-		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		cc.assertCalls("Shutdown", "unpause", "New", "Login", "unpause")
 		c.Assert(nn[0].Prefs, qt.IsNotNil)
 		c.Assert(nn[1].State, qt.IsNotNil)
 		c.Assert(nn[0].Prefs.WantRunning, qt.IsFalse)
@ -760,7 +758,7 @@ func TestStateMachine(t *testing.T) {
 	})
 	{
 		notifies.drain(0)
-		c.Assert([]string{"pause", "pause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("pause", "pause")
 	}

 	// Request connection.
@ -773,7 +771,7 @@ func TestStateMachine(t *testing.T) {
 	})
 	{
 		nn := notifies.drain(2)
-		c.Assert([]string{"Login", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("Login", "unpause")
 		// BUG: I would expect Prefs to change first, and state after.
 		c.Assert(nn[0].State, qt.IsNotNil)
 		c.Assert(nn[1].Prefs, qt.IsNotNil)
@ -789,7 +787,7 @@ func TestStateMachine(t *testing.T) {
 	})
 	{
 		nn := notifies.drain(2)
-		c.Assert([]string{"pause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("pause")
 		// BUG: I would expect Prefs to change first, and state after.
 		c.Assert(nn[0].State, qt.IsNotNil)
 		c.Assert(nn[1].Prefs, qt.IsNotNil)
@ -812,7 +810,7 @@ func TestStateMachine(t *testing.T) {
 		//
 		// Because the login hasn't yet completed, the old login
 		// is still valid, so it's correct that we stay paused.
-		c.Assert([]string{"Login", "pause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("Login", "pause")
 		c.Assert(nn[0].BrowseToURL, qt.IsNotNil)
 		c.Assert(*nn[0].BrowseToURL, qt.Equals, url3)
 	}
@ -834,7 +832,7 @@ func TestStateMachine(t *testing.T) {
 		//  and !WantRunning. But since it's a fresh and successful
 		//  new login, WantRunning is true, so there was never a
 		//  reason to pause().
-		c.Assert([]string{"pause", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("pause", "unpause")
 		c.Assert(nn[0].LoginFinished, qt.IsNotNil)
 		c.Assert(nn[1].Prefs, qt.IsNotNil)
 		c.Assert(nn[2].State, qt.IsNotNil)
@ -853,10 +851,10 @@ func TestStateMachine(t *testing.T) {
 	{
 		// NOTE: cc.Shutdown() is correct here, since we didn't call
 		// b.Shutdown() ourselves.
-		c.Assert([]string{"Shutdown", "unpause", "New", "Login", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("Shutdown", "unpause", "New", "Login", "unpause")

 		nn := notifies.drain(1)
-		c.Assert(cc.getCalls(), qt.HasLen, 0)
+		cc.assertCalls()
 		c.Assert(nn[0].Prefs, qt.IsNotNil)
 		c.Assert(nn[0].Prefs.LoggedOut, qt.IsFalse)
 		c.Assert(nn[0].Prefs.WantRunning, qt.IsTrue)
@ -872,7 +870,7 @@ func TestStateMachine(t *testing.T) {
 	})
 	{
 		nn := notifies.drain(1)
-		c.Assert([]string{"unpause", "unpause"}, qt.DeepEquals, cc.getCalls())
+		cc.assertCalls("unpause", "unpause")
 		// NOTE: No LoginFinished message since no interactive
 		// login was needed.
 		c.Assert(nn[0].State, qt.IsNotNil)
@ -920,7 +918,7 @@ func TestWGEngineStatusRace(t *testing.T) {
 	b, err := NewLocalBackend(logf, "logid", new(ipn.MemoryStore), eng)
 	c.Assert(err, qt.IsNil)

-	cc := newMockControl()
+	cc := newMockControl(t)
 	b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) {
 		cc.mu.Lock()
 		defer cc.mu.Unlock()