TheArchive/tailscale
1
0
Fork 0
mirror of https://github.com/tailscale/tailscale.git synced 2025-03-15 09:50:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

ipn/ipnlocal: do not shut down the backend halfway through TestStateMachine

Browse Source 
LocalBackend.Shutdown's docs say:

> The backend can no longer be used after Shutdown returns.

Nevertheless, TestStateMachine blithely calls Shutdown, talks some smack,
and continues on, expecting things to work. Other uses of Shutdown
in the codebase are as intended.

Things mostly kinda work anyway, except that the wgengine.Engine has been
shut down, so calls to Reconfig fail. Those get logged:

> local.go:603: wgengine status error: engine closing; no status

but otherwise ignored.

However, the Reconfig failure caused one fewer call to pause/unpause
than normal. Now the assertCalls lines match the equivalent ones
earlier in the test.

I don't see an obvious correct replacement for Shutdown in the context
of this test; I'm not sure entirely what it is trying to accomplish.
It is possible that many of the tests remaining after the prior call
to Shutdown are now extraneous. They don't harm anything, though,
so err on the side of safety and leave them for now.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
This commit is contained in:
Josh Bleecher Snyder 2021-09-15 16:40:54 -07:00 committed by Josh Bleecher Snyder
parent f834a4ade5
commit 07c09f470d
1 changed files with 5 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
ipn/ipnlocal/state_test.go
View File

@ -645,16 +645,6 @@ func TestStateMachine(t *testing.T) {
c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
}
// Shut down the backend.
t.Logf("\n\nShutdown")
notifies.expect(0)
b.Shutdown()
{
notifies.drain(0)
// BUG: I expect a transition to ipn.NoState here.
cc.assertCalls("Shutdown")
}
// Oh, you thought we were done? Ha! Now we have to test what
// happens if the user exits and restarts while logged out.
// Note that it's explicitly okay to call b.Start() over and over
@ -694,7 +684,7 @@ func TestStateMachine(t *testing.T) {
})
{
nn := notifies.drain(3)
cc.assertCalls("unpause", "unpause")
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)
@ -771,7 +761,7 @@ func TestStateMachine(t *testing.T) {
})
{
nn := notifies.drain(2)
cc.assertCalls("Login", "unpause")
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)
@ -810,7 +800,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.
cc.assertCalls("Login", "pause")
cc.assertCalls("Login", "pause", "pause")
c.Assert(nn[0].BrowseToURL, qt.IsNotNil)
c.Assert(*nn[0].BrowseToURL, qt.Equals, url3)
}
@ -832,7 +822,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().
cc.assertCalls("pause", "unpause")
cc.assertCalls("pause", "unpause", "unpause")
c.Assert(nn[0].LoginFinished, qt.IsNotNil)
c.Assert(nn[1].Prefs, qt.IsNotNil)
c.Assert(nn[2].State, qt.IsNotNil)
@ -870,7 +860,7 @@ func TestStateMachine(t *testing.T) {
})
{
nn := notifies.drain(1)
cc.assertCalls("unpause", "unpause")
cc.assertCalls("unpause", "unpause", "unpause")
// NOTE: No LoginFinished message since no interactive
// login was needed.
c.Assert(nn[0].State, qt.IsNotNil)