Skip to content

Commit

Permalink
Make shutdown timeout configurable for tests
Browse files Browse the repository at this point in the history
TestWebauthnConfigChanges seems to need a longer shutdown timeout when running
on GitHub Actions. The problem manifests in errors like this:

```
2024-05-05T17:00:45.0503602Z     api_test.go:2919: TestWebauthnConfigChanges/Can_edit_GUIConfiguration.WebauthnUserId Put "http://127.0.0.1:37585/rest/config/gui": EOF []
2024-05-05T17:00:45.0566336Z --- FAIL: TestWebauthnConfigChanges/Can_edit_GUIConfiguration.WebauthnUserId (0.52s)
```

indicating that the server was forcefully shut down (or panicked, but that was
ruled out in these cases) before the request was fully written.
  • Loading branch information
emlun committed May 12, 2024
1 parent e841612 commit 7ed2db8
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/api/api.go
Expand Up @@ -93,6 +93,7 @@ type service struct {
listenerAddr net.Addr
exitChan chan *svcutil.FatalErr
miscDB *db.NamespacedKV
shutdownTimeout time.Duration

guiErrors logger.Recorder
systemLog logger.Recorder
Expand Down Expand Up @@ -130,6 +131,7 @@ func New(id protocol.DeviceID, cfg config.Wrapper, assetDir, tlsDefaultCommonNam
startedOnce: make(chan struct{}),
exitChan: make(chan *svcutil.FatalErr, 1),
miscDB: miscDB,
shutdownTimeout: 100 * time.Millisecond,
}
}

Expand Down Expand Up @@ -458,7 +460,7 @@ func (s *service) Serve(ctx context.Context) error {
}
// Give it a moment to shut down gracefully, e.g. if we are restarting
// due to a config change through the API, let that finish successfully.
timeout, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
timeout, cancel := context.WithTimeout(context.Background(), s.shutdownTimeout)
defer cancel()
if err := srv.Shutdown(timeout); err == timeout.Err() {
srv.Close()
Expand Down
8 changes: 8 additions & 0 deletions lib/api/api_test.go
Expand Up @@ -957,6 +957,10 @@ func TestApiCache(t *testing.T) {
}

func startHTTP(cfg config.Wrapper) (string, context.CancelFunc, error) {
return startHTTPWithShutdownTimeout(cfg, 0)
}

func startHTTPWithShutdownTimeout(cfg config.Wrapper, shutdownTimeout time.Duration) (string, context.CancelFunc, error) {
m := new(modelmocks.Model)
assetDir := "../../gui"
eventSub := new(eventmocks.BufferedSubscription)
Expand Down Expand Up @@ -984,6 +988,10 @@ func startHTTP(cfg config.Wrapper) (string, context.CancelFunc, error) {
svc := New(protocol.LocalDeviceID, cfg, assetDir, "syncthing", m, eventSub, diskEventSub, events.NoopLogger, discoverer, connections, urService, mockedSummary, errorLog, systemLog, false, kdb).(*service)
svc.started = addrChan

if shutdownTimeout > 0*time.Millisecond {
svc.shutdownTimeout = shutdownTimeout
}

// Actually start the API service
supervisor := suture.New("API test", suture.Spec{
PassThroughPanics: true,
Expand Down
9 changes: 9 additions & 0 deletions lib/testutil/testutil.go
Expand Up @@ -8,6 +8,7 @@ package testutil

import (
"errors"
"os"
"sync"
"testing"

Expand Down Expand Up @@ -63,6 +64,14 @@ func (NoopCloser) Close() error {
return nil
}

func IfNotCI[T any](notCiValue T, ciValue T) T {
if os.Getenv("CI") == "true" {
return ciValue
} else {
return notCiValue
}
}

func ConcatSlices[T any](slices ...[]T) []T {
// TODO when go >= 1.22.0: Replace with slices.Concat
concatLen := 0
Expand Down

0 comments on commit 7ed2db8

Please sign in to comment.