Skip to content

fix: isolate keyring usage by parallel test processes #21256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zedkipp merged 1 commit into from
Dec 15, 2025
+27 −34
Merged

fix: isolate keyring usage by parallel test processes #21256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 2 additions & 17 deletions cli/keyring_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,11 @@ package cli_test

import (
"bytes"
"crypto/rand"
"encoding/binary"
"fmt"
"net/url"
"os"
"path"
"runtime"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -19,23 +15,12 @@ import (
"github.com/coder/coder/v2/cli/clitest"
"github.com/coder/coder/v2/cli/config"
"github.com/coder/coder/v2/cli/sessionstore"
"github.com/coder/coder/v2/cli/sessionstore/testhelpers"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/pty/ptytest"
"github.com/coder/serpent"
)

// keyringTestServiceName generates a unique service name for keyring tests
// using the test name and a nanosecond timestamp to prevent collisions.
func keyringTestServiceName(t *testing.T) string {
t.Helper()
var n uint32
err := binary.Read(rand.Reader, binary.BigEndian, &n)
if err != nil {
t.Fatal(err)
}
return fmt.Sprintf("%s_%v_%d", t.Name(), time.Now().UnixNano(), n)
Copy link
Member

@ethanndickson ethanndickson Dec 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flake was on Windows, and the Windows clock does have infamously poor precision (https://github.com/search?q=org%3Acoder+windows+clock+flake&type=issues, so I'm inclined to believe your hypothesis of multiple instances of the job running on the same Windows machine.

}

type keyringTestEnv struct {
serviceName string
keyring sessionstore.Keyring
Expand All @@ -52,7 +37,7 @@ func setupKeyringTestEnv(t *testing.T, clientURL string, args ...string) keyring
cmd, err := root.Command(root.AGPL())
require.NoError(t, err)

serviceName := keyringTestServiceName(t)
serviceName := testhelpers.KeyringServiceName(t)
root.WithKeyringServiceName(serviceName)
root.UseKeyringWithGlobalConfig()

Expand Down
24 changes: 8 additions & 16 deletions cli/sessionstore/sessionstore_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,24 @@ package sessionstore_test
import (
"encoding/json"
"errors"
"fmt"
"net/url"
"os"
"path"
"runtime"
"testing"
"time"

"github.com/stretchr/testify/require"

"github.com/coder/coder/v2/cli/config"
"github.com/coder/coder/v2/cli/sessionstore"
"github.com/coder/coder/v2/cli/sessionstore/testhelpers"
)

type storedCredentials map[string]struct {
CoderURL string `json:"coder_url"`
APIToken string `json:"api_token"`
}

// Generate a test service name for use with the OS keyring. It uses a combination
// of the test name and a nanosecond timestamp to prevent collisions.
func keyringTestServiceName(t *testing.T) string {
t.Helper()
return t.Name() + "_" + fmt.Sprintf("%v", time.Now().UnixNano())
}

func TestKeyring(t *testing.T) {
t.Parallel()

Expand All @@ -47,7 +39,7 @@ func TestKeyring(t *testing.T) {
t.Run("ReadNonExistent", func(t *testing.T) {
t.Parallel()

backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
srvURL, err := url.Parse(testURL)
require.NoError(t, err)
t.Cleanup(func() { _ = backend.Delete(srvURL) })
Expand All @@ -60,7 +52,7 @@ func TestKeyring(t *testing.T) {
t.Run("DeleteNonExistent", func(t *testing.T) {
t.Parallel()

backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
srvURL, err := url.Parse(testURL)
require.NoError(t, err)
t.Cleanup(func() { _ = backend.Delete(srvURL) })
Expand All @@ -73,7 +65,7 @@ func TestKeyring(t *testing.T) {
t.Run("WriteAndRead", func(t *testing.T) {
t.Parallel()

backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
srvURL, err := url.Parse(testURL)
require.NoError(t, err)
t.Cleanup(func() { _ = backend.Delete(srvURL) })
Expand Down Expand Up @@ -101,7 +93,7 @@ func TestKeyring(t *testing.T) {
t.Run("WriteAndDelete", func(t *testing.T) {
t.Parallel()

backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
srvURL, err := url.Parse(testURL)
require.NoError(t, err)
t.Cleanup(func() { _ = backend.Delete(srvURL) })
Expand All @@ -125,7 +117,7 @@ func TestKeyring(t *testing.T) {
t.Run("OverwriteToken", func(t *testing.T) {
t.Parallel()

backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
srvURL, err := url.Parse(testURL)
require.NoError(t, err)
t.Cleanup(func() { _ = backend.Delete(srvURL) })
Expand Down Expand Up @@ -156,7 +148,7 @@ func TestKeyring(t *testing.T) {
t.Run("MultipleServers", func(t *testing.T) {
t.Parallel()

backend := sessionstore.NewKeyringWithService(keyringTestServiceName(t))
backend := sessionstore.NewKeyringWithService(testhelpers.KeyringServiceName(t))
srvURL, err := url.Parse(testURL)
require.NoError(t, err)
srvURL2, err := url.Parse(testURL2)
Expand Down Expand Up @@ -220,7 +212,7 @@ func TestKeyring(t *testing.T) {
srv2URL, err := url.Parse(testURL2)
require.NoError(t, err)

serviceName := keyringTestServiceName(t)
serviceName := testhelpers.KeyringServiceName(t)
backend := sessionstore.NewKeyringWithService(serviceName)
t.Cleanup(func() {
_ = backend.Delete(srv1URL)
Expand Down
3 changes: 2 additions & 1 deletion cli/sessionstore/sessionstore_windows_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/coder/coder/v2/cli/sessionstore"
"github.com/coder/coder/v2/cli/sessionstore/testhelpers"
)

func readRawKeychainCredential(t *testing.T, serviceName string) []byte {
Expand All @@ -31,7 +32,7 @@ func TestWindowsKeyring_WriteReadDelete(t *testing.T) {
srvURL, err := url.Parse(testURL)
require.NoError(t, err)

serviceName := keyringTestServiceName(t)
serviceName := testhelpers.KeyringServiceName(t)
backend := sessionstore.NewKeyringWithService(serviceName)
t.Cleanup(func() { _ = backend.Delete(srvURL) })

Expand Down
15 changes: 15 additions & 0 deletions cli/sessionstore/testhelpers/testhelpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package testhelpers

import (
"fmt"
"os"
"testing"
)

// KeyringServiceName generates a test service name for use with the OS keyring.
// It intends to prevent keyring usage collisions between parallel tests within a
// process and parallel test processes (which may occur on CI).
func KeyringServiceName(t *testing.T) string {
t.Helper()
return t.Name() + "_" + fmt.Sprintf("%v", os.Getpid())
}
Loading