TheArchive/tailscale
1
0
Fork 0
mirror of https://github.com/tailscale/tailscale.git synced 2024-11-29 13:05:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

ipn/ipnlocal: fix test flake when we log after a test completes

Browse Source 
This switches from using an atomic.Bool to a mutex for reasons that are
described in the commit, and should address the flakes that we're still
seeing.

Fixes #3020

Change-Id: I4e39471c0eb95886db03020ea1ccf688c7564a11
Signed-off-by: Andrew Dunham <andrew@tailscale.com>
This commit is contained in:
Andrew Dunham 2022-11-02 15:45:20 -04:00 committed by Andrew Dunham
parent 0af57fce4c
commit e975cb6b05
2 changed files with 52 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
ipn/ipnlocal/state_test.go
View File

@ -17,6 +17,7 @@
"tailscale.com/ipn"
"tailscale.com/ipn/store/mem"
"tailscale.com/tailcfg"
"tailscale.com/tstest"
"tailscale.com/types/empty"
"tailscale.com/types/key"
"tailscale.com/types/logger"
@ -92,7 +93,6 @@ type mockControl struct {
opts controlclient.Options
logfActual logger.Logf
statusFunc func(controlclient.Status)
preventLog atomic.Bool
mu sync.Mutex
calls []string
@ -109,7 +109,7 @@ func newMockControl(tb testing.TB) *mockControl {
}
func (cc *mockControl) logf(format string, args ...any) {
if cc.preventLog.Load() || cc.logfActual == nil {
if cc.logfActual == nil {
return
}
cc.logfActual(format, args...)
@ -282,7 +282,7 @@ func (cc *mockControl) UpdateEndpoints(endpoints []tailcfg.Endpoint) {
func TestStateMachine(t *testing.T) {
c := qt.New(t)
logf := t.Logf
logf := tstest.WhileTestRunningLogger(t)
store := new(testStateStorage)
e, err := wgengine.NewFakeUserspaceEngine(logf, 0)
if err != nil {
@ -297,7 +297,6 @@ func TestStateMachine(t *testing.T) {
cc := newMockControl(t)
cc.statusFunc = b.setClientStatus
t.Cleanup(func() { cc.preventLog.Store(true) }) // hacky way to pacify issue 3020
b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) {
cc.mu.Lock()
@ -316,9 +315,6 @@ func TestStateMachine(t *testing.T) {
notifies.expect(0)
b.SetNotifyCallback(func(n ipn.Notify) {
if cc.preventLog.Load() {
return
}
if n.State != nil ||
n.Prefs.Valid() ||
n.BrowseToURL != nil ||
@ -329,7 +325,6 @@ func TestStateMachine(t *testing.T) {
logf("\n(ignored) %v\n\n", n)
}
})
t.Cleanup(func() { b.SetNotifyCallback(nil) }) // hacky way to pacify issue 3020
// Check that it hasn't called us right away.
// The state machine should be idle until we call Start().
@ -924,7 +919,7 @@ func TestStateMachine(t *testing.T) {
}
func TestEditPrefsHasNoKeys(t *testing.T) {
logf := t.Logf
logf := tstest.WhileTestRunningLogger(t)
store := new(testStateStorage)
e, err := wgengine.NewFakeUserspaceEngine(logf, 0)
if err != nil {
@ -1006,7 +1001,7 @@ func (s *testStateStorage) sawWrite() bool {
func TestWGEngineStatusRace(t *testing.T) {
t.Skip("test fails")
c := qt.New(t)
logf := t.Logf
logf := tstest.WhileTestRunningLogger(t)
eng, err := wgengine.NewFakeUserspaceEngine(logf, 0)
c.Assert(err, qt.IsNil)
t.Cleanup(eng.Close)

47
tstest/log.go
View File

@ -145,3 +145,50 @@ func (ml *MemLogger) String() string {
defer ml.Unlock()
return ml.Buffer.String()
}
// WhileTestRunningLogger returns a logger.Logf that logs to t.Logf until the
// test finishes, at which point it no longer logs anything.
func WhileTestRunningLogger(t testing.TB) logger.Logf {
var (
mu sync.RWMutex
done bool
)
logger := func(format string, args ...any) {
t.Helper()
mu.RLock()
defer mu.RUnlock()
if done {
return
}
t.Logf(format, args...)
}
// t.Cleanup is run before the test is marked as done, so by acquiring
// the mutex and then disabling logs, we know that all existing log
// functions have completed, and that no future calls to the logger
// will log something.
//
// We can't do this with an atomic bool, since it's possible to
// observe the following race:
//
// test goroutine goroutine 1
// -------------- -----------
// check atomic, testFinished = no
// test finishes
// run t.Cleanups
// set testFinished = true
// call t.Logf
// panic
//
// Using a mutex ensures that all actions in goroutine 1 in the
// sequence above occur atomically, and thus should not panic.
t.Cleanup(func() {
mu.Lock()
defer mu.Unlock()
done = true
})
return logger
}