Fix a bug in the e2e tests

When the test was going to fail, a goroutine would accidentally block
on writing to an unbuffered channel, and the spawnTestGoroutine helper
would wait for that goroutine to end on cleanup, causing the test to
hang forever while it was trying to fail.

(cherry picked from commit 2b93fdf357)
This commit is contained in:
Ryan Richard 2022-02-07 11:57:54 -08:00 committed by Margo Crawford
parent 19ec85c84e
commit bb71545dee
2 changed files with 10 additions and 9 deletions

View File

@ -1,4 +1,4 @@
// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. // Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 // SPDX-License-Identifier: Apache-2.0
package integration package integration
@ -292,7 +292,7 @@ func runPinnipedLoginOIDC(
}) })
// Start a background goroutine to read stderr from the CLI and parse out the login URL. // Start a background goroutine to read stderr from the CLI and parse out the login URL.
loginURLChan := make(chan string) loginURLChan := make(chan string, 1)
spawnTestGoroutine(t, func() (err error) { spawnTestGoroutine(t, func() (err error) {
t.Helper() t.Helper()
defer func() { defer func() {
@ -311,7 +311,7 @@ func runPinnipedLoginOIDC(
for scanner.Scan() { for scanner.Scan() {
loginURL, err := url.Parse(strings.TrimSpace(scanner.Text())) loginURL, err := url.Parse(strings.TrimSpace(scanner.Text()))
if err == nil && loginURL.Scheme == "https" { if err == nil && loginURL.Scheme == "https" {
loginURLChan <- loginURL.String() loginURLChan <- loginURL.String() // this channel is buffered so this will not block
return nil return nil
} }
} }
@ -320,7 +320,7 @@ func runPinnipedLoginOIDC(
}) })
// Start a background goroutine to read stdout from the CLI and parse out an ExecCredential. // Start a background goroutine to read stdout from the CLI and parse out an ExecCredential.
credOutputChan := make(chan clientauthenticationv1beta1.ExecCredential) credOutputChan := make(chan clientauthenticationv1beta1.ExecCredential, 1)
spawnTestGoroutine(t, func() (err error) { spawnTestGoroutine(t, func() (err error) {
defer func() { defer func() {
closeErr := stdout.Close() closeErr := stdout.Close()
@ -336,7 +336,7 @@ func runPinnipedLoginOIDC(
if err := json.NewDecoder(reader).Decode(&out); err != nil { if err := json.NewDecoder(reader).Decode(&out); err != nil {
return fmt.Errorf("could not read ExecCredential from stdout: %w", err) return fmt.Errorf("could not read ExecCredential from stdout: %w", err)
} }
credOutputChan <- out credOutputChan <- out // this channel is buffered so this will not block
return readAndExpectEmpty(reader) return readAndExpectEmpty(reader)
}) })
@ -391,6 +391,7 @@ func readAndExpectEmpty(r io.Reader) (err error) {
return nil return nil
} }
// Note: Callers should ensure that f eventually returns, otherwise this helper will hang forever in t.Cleanup.
func spawnTestGoroutine(t *testing.T, f func() error) { func spawnTestGoroutine(t *testing.T, f func() error) {
t.Helper() t.Helper()
var eg errgroup.Group var eg errgroup.Group

View File

@ -182,7 +182,7 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo
}) })
// Start a background goroutine to read stderr from the CLI and parse out the login URL. // Start a background goroutine to read stderr from the CLI and parse out the login URL.
loginURLChan := make(chan string) loginURLChan := make(chan string, 1)
spawnTestGoroutine(t, func() (err error) { spawnTestGoroutine(t, func() (err error) {
defer func() { defer func() {
closeErr := stderrPipe.Close() closeErr := stderrPipe.Close()
@ -199,7 +199,7 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo
for scanner.Scan() { for scanner.Scan() {
loginURL, err := url.Parse(strings.TrimSpace(scanner.Text())) loginURL, err := url.Parse(strings.TrimSpace(scanner.Text()))
if err == nil && loginURL.Scheme == "https" { if err == nil && loginURL.Scheme == "https" {
loginURLChan <- loginURL.String() loginURLChan <- loginURL.String() // this channel is buffered so this will not block
return nil return nil
} }
} }
@ -207,7 +207,7 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo
}) })
// Start a background goroutine to read stdout from kubectl and return the result as a string. // Start a background goroutine to read stdout from kubectl and return the result as a string.
kubectlOutputChan := make(chan string) kubectlOutputChan := make(chan string, 1)
spawnTestGoroutine(t, func() (err error) { spawnTestGoroutine(t, func() (err error) {
defer func() { defer func() {
closeErr := stdoutPipe.Close() closeErr := stdoutPipe.Close()
@ -223,7 +223,7 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo
return err return err
} }
t.Logf("kubectl output:\n%s\n", output) t.Logf("kubectl output:\n%s\n", output)
kubectlOutputChan <- string(output) kubectlOutputChan <- string(output) // this channel is buffered so this will not block
return nil return nil
}) })