coder · Kira-Pilot · Jul 20, 2022 · Jul 20, 2022 · Jul 20, 2022 · Jul 20, 2022
diff --git a/.github/workflows/coder.yaml b/.github/workflows/coder.yaml
@@ -6,6 +6,8 @@ on:
       - main
     tags:
       - "*"
+    paths-ignore:
+      - "docs/**"
 
   pull_request:
 
@@ -217,18 +219,22 @@ jobs:
         run: go run scripts/datadog-cireport/main.go gotests.xml
 
       - uses: codecov/codecov-action@v3
+        # This action has a tendency to error out unexpectedly, it has
+        # the `fail_ci_if_error` option that defaults to `false`, but
+        # that is no guarantee, see:
+        # https://github.com/codecov/codecov-action/issues/788
+        continue-on-error: true
         if: github.actor != 'dependabot[bot]' && !github.event.pull_request.head.repo.fork
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           files: ./gotests.coverage
           flags: unittest-go-${{ matrix.os }}
-          # this flakes and sometimes fails the build
-          fail_ci_if_error: false
 
   test-go-postgres:
     name: "test/go/postgres"
     runs-on: ubuntu-latest
-    timeout-minutes: 20
+    # This timeout must be greater than go test -timeout.
+    timeout-minutes: 25
     steps:
       - uses: actions/checkout@v3
 
@@ -279,13 +285,16 @@ jobs:
         run: go run scripts/datadog-cireport/main.go gotests.xml
 
       - uses: codecov/codecov-action@v3
+        # This action has a tendency to error out unexpectedly, it has
+        # the `fail_ci_if_error` option that defaults to `false`, but
+        # that is no guarantee, see:
+        # https://github.com/codecov/codecov-action/issues/788
+        continue-on-error: true
         if: github.actor != 'dependabot[bot]' && !github.event.pull_request.head.repo.fork
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           files: ./gotests.coverage
           flags: unittest-go-postgres-${{ matrix.os }}
-          # this flakes and sometimes fails the build
-          fail_ci_if_error: false
 
   deploy:
     name: "deploy"
@@ -429,13 +438,16 @@ jobs:
         working-directory: site
 
       - uses: codecov/codecov-action@v3
+        # This action has a tendency to error out unexpectedly, it has
+        # the `fail_ci_if_error` option that defaults to `false`, but
+        # that is no guarantee, see:
+        # https://github.com/codecov/codecov-action/issues/788
+        continue-on-error: true
         if: github.actor != 'dependabot[bot]' && !github.event.pull_request.head.repo.fork
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           files: ./site/coverage/lcov.info
           flags: unittest-js
-          # this flakes and sometimes fails the build
-          fail_ci_if_error: false
 
       - name: Upload DataDog Trace
         if: always() && github.actor != 'dependabot[bot]' && !github.event.pull_request.head.repo.fork

diff --git a/.github/workflows/stale.yaml b/.github/workflows/stale.yaml
@@ -12,5 +12,6 @@ jobs:
       pull-requests: write
     steps:
       - uses: actions/stale@v5.1.0
-        stale-issue-label: stale
-        stale-pr-label: stale
+        with:
+          stale-issue-label: stale
+          stale-pr-label: stale
diff --git a/cli/cliflag/cliflag.go b/cli/cliflag/cliflag.go
@@ -17,9 +17,33 @@ import (
 	"strings"
 	"time"
 
+	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
 
+// IsSetBool returns the value of the boolean flag if it is set.
+// It returns false if the flag isn't set or if any error occurs attempting
+// to parse the value of the flag.
+func IsSetBool(cmd *cobra.Command, name string) bool {
+	val, ok := IsSet(cmd, name)
+	if !ok {
+		return false
+	}
+
+	b, err := strconv.ParseBool(val)
+	return err == nil && b
+}
+
+// IsSet returns the string value of the flag and whether it was set.
+func IsSet(cmd *cobra.Command, name string) (string, bool) {
+	flag := cmd.Flag(name)
+	if flag == nil {
+		return "", false
+	}
+
+	return flag.Value.String(), flag.Changed
+}
+
 // String sets a string flag on the given flag set.
 func String(flagset *pflag.FlagSet, name, shorthand, env, def, usage string) {
 	v, ok := os.LookupEnv(env)
@@ -67,6 +91,22 @@ func Uint8VarP(flagset *pflag.FlagSet, ptr *uint8, name string, shorthand string
 	flagset.Uint8VarP(ptr, name, shorthand, uint8(vi64), fmtUsage(usage, env))
 }
 
+func Bool(flagset *pflag.FlagSet, name, shorthand, env string, def bool, usage string) {
+	val, ok := os.LookupEnv(env)
+	if !ok || val == "" {
+		flagset.BoolP(name, shorthand, def, fmtUsage(usage, env))
+		return
+	}
+
+	valb, err := strconv.ParseBool(val)
+	if err != nil {
+		flagset.BoolP(name, shorthand, def, fmtUsage(usage, env))
+		return
+	}
+
+	flagset.BoolP(name, shorthand, valb, fmtUsage(usage, env))
+}
+
 // BoolVarP sets a bool flag on the given flag set.
 func BoolVarP(flagset *pflag.FlagSet, ptr *bool, name string, shorthand string, env string, def bool, usage string) {
 	val, ok := os.LookupEnv(env)

diff --git a/cli/create_test.go b/cli/create_test.go
@@ -88,7 +88,7 @@ func TestCreate(t *testing.T) {
 
 		member := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
 		clitest.SetupConfig(t, member, root)
-		cmdCtx, done := context.WithTimeout(context.Background(), time.Second*3)
+		cmdCtx, done := context.WithTimeout(context.Background(), 10*time.Second)
 		go func() {
 			defer done()
 			err := cmd.ExecuteContext(cmdCtx)

diff --git a/cli/login.go b/cli/login.go
@@ -73,7 +73,9 @@ func login() *cobra.Command {
 			// on a very old client.
 			err = checkVersions(cmd, client)
 			if err != nil {
-				return xerrors.Errorf("check versions: %w", err)
+				// Checking versions isn't a fatal error so we print a warning
+				// and proceed.
+				_, _ = fmt.Fprintln(cmd.ErrOrStderr(), cliui.Styles.Warn.Render(err.Error()))
 			}
 
 			hasInitialUser, err := client.HasFirstUser(cmd.Context())

diff --git a/cli/portforward_test.go b/cli/portforward_test.go
@@ -142,21 +142,22 @@ func TestPortForward(t *testing.T) {
 		},
 	}
 
+	// Setup agent once to be shared between test-cases (avoid expensive
+	// non-parallel setup).
+	var (
+		client       = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
+		user         = coderdtest.CreateFirstUser(t, client)
+		_, workspace = runAgent(t, client, user.UserID)
+	)
+
 	for _, c := range cases { //nolint:paralleltest // the `c := c` confuses the linter
 		c := c
-		// Avoid parallel test here because setupLocal reserves
+		// Delay parallel tests here because setupLocal reserves
 		// a free open port which is not guaranteed to be free
-		// after the listener closes.
-		//nolint:paralleltest
+		// between the listener closing and port-forward ready.
 		t.Run(c.name, func(t *testing.T) {
-			//nolint:paralleltest
 			t.Run("OnePort", func(t *testing.T) {
-				var (
-					client       = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
-					user         = coderdtest.CreateFirstUser(t, client)
-					_, workspace = runAgent(t, client, user.UserID)
-					p1           = setupTestListener(t, c.setupRemote(t))
-				)
+				p1 := setupTestListener(t, c.setupRemote(t))
 
 				// Create a flag that forwards from local to listener 1.
 				localAddress, localFlag := c.setupLocal(t)
@@ -176,6 +177,8 @@ func TestPortForward(t *testing.T) {
 				}()
 				waitForPortForwardReady(t, buf)
 
+				t.Parallel() // Port is reserved, enable parallel execution.
+
 				// Open two connections simultaneously and test them out of
 				// sync.
 				d := net.Dialer{Timeout: 3 * time.Second}
@@ -196,11 +199,8 @@ func TestPortForward(t *testing.T) {
 			//nolint:paralleltest
 			t.Run("TwoPorts", func(t *testing.T) {
 				var (
-					client       = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
-					user         = coderdtest.CreateFirstUser(t, client)
-					_, workspace = runAgent(t, client, user.UserID)
-					p1           = setupTestListener(t, c.setupRemote(t))
-					p2           = setupTestListener(t, c.setupRemote(t))
+					p1 = setupTestListener(t, c.setupRemote(t))
+					p2 = setupTestListener(t, c.setupRemote(t))
 				)
 
 				// Create a flags for listener 1 and listener 2.
@@ -223,6 +223,8 @@ func TestPortForward(t *testing.T) {
 				}()
 				waitForPortForwardReady(t, buf)
 
+				t.Parallel() // Port is reserved, enable parallel execution.
+
 				// Open a connection to both listener 1 and 2 simultaneously and
 				// then test them out of order.
 				d := net.Dialer{Timeout: 3 * time.Second}
@@ -246,10 +248,6 @@ func TestPortForward(t *testing.T) {
 	//nolint:paralleltest
 	t.Run("TCP2Unix", func(t *testing.T) {
 		var (
-			client       = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
-			user         = coderdtest.CreateFirstUser(t, client)
-			_, workspace = runAgent(t, client, user.UserID)
-
 			// Find the TCP and Unix cases so we can use their setupLocal and
 			// setupRemote methods respectively.
 			tcpCase  = cases[0]
@@ -278,6 +276,8 @@ func TestPortForward(t *testing.T) {
 		}()
 		waitForPortForwardReady(t, buf)
 
+		t.Parallel() // Port is reserved, enable parallel execution.
+
 		// Open two connections simultaneously and test them out of
 		// sync.
 		d := net.Dialer{Timeout: 3 * time.Second}
@@ -299,9 +299,6 @@ func TestPortForward(t *testing.T) {
 	//nolint:paralleltest
 	t.Run("All", func(t *testing.T) {
 		var (
-			client       = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
-			user         = coderdtest.CreateFirstUser(t, client)
-			_, workspace = runAgent(t, client, user.UserID)
 			// These aren't fixed size because we exclude Unix on Windows.
 			dials = []addr{}
 			flags = []string{}
@@ -339,6 +336,8 @@ func TestPortForward(t *testing.T) {
 		}()
 		waitForPortForwardReady(t, buf)
 
+		t.Parallel() // Port is reserved, enable parallel execution.
+
 		// Open connections to all items in the "dial" array.
 		var (
 			d     = net.Dialer{Timeout: 3 * time.Second}
@@ -425,6 +424,8 @@ func runAgent(t *testing.T, client *codersdk.Client, userID uuid.UUID) ([]coders
 // setupTestListener starts accepting connections and echoing a single packet.
 // Returns the listener and the listen port or Unix path.
 func setupTestListener(t *testing.T, l net.Listener) string {
+	t.Helper()
+
 	// Wait for listener to completely exit before releasing.
 	done := make(chan struct{})
 	t.Cleanup(func() {
@@ -440,6 +441,7 @@ func setupTestListener(t *testing.T, l net.Listener) string {
 		for {
 			c, err := l.Accept()
 			if err != nil {
+				_ = l.Close()
 				return
 			}
 
@@ -479,6 +481,7 @@ func testAccept(t *testing.T, c net.Conn) {
 }
 
 func assertReadPayload(t *testing.T, r io.Reader, payload []byte) {
+	t.Helper()
 	b := make([]byte, len(payload)+16)
 	n, err := r.Read(b)
 	assert.NoError(t, err, "read payload")
@@ -487,12 +490,14 @@ func assertReadPayload(t *testing.T, r io.Reader, payload []byte) {
 }
 
 func assertWritePayload(t *testing.T, w io.Writer, payload []byte) {
+	t.Helper()
 	n, err := w.Write(payload)
 	assert.NoError(t, err, "write payload")
 	assert.Equal(t, len(payload), n, "payload length does not match")
 }
 
 func waitForPortForwardReady(t *testing.T, output *threadSafeBuffer) {
+	t.Helper()
 	for i := 0; i < 100; i++ {
 		time.Sleep(250 * time.Millisecond)