fix: Added scenarios for telemetry disabled-default

Telemetry is disabled when:
 - os.Getenv("CI") == "true" beacuse it means we are in a CI environment
   and we don't want to ask for telemetry approval
 - stdout is not a TTY because we can not ask for telemetry approval.
   Plus it also implies we are in a script context and we may not want
   to be prompt anything

.circleci/config.yml

@@ -137,6 +137,10 @@ jobs:
           working_directory: integration_tests
           environment:
             TESTING: "true"
+      - run:
+          name: "Make sure simple command do not cause any timeout"
+          command: circleci version
+
   test:
     executor: go
     steps:
@@ -378,12 +382,14 @@ workflows:
             - devex-release
       - deploy:
           requires:
+            - cucumber
             - test
             - test_mac
             - coverage
             - lint
             - deploy-test
             - shellcheck/check
+            - vulnerability-scan
           filters:
             branches:
               only: main

cmd/create_telemetry.go

@@ -20,6 +20,7 @@ import (
 var (
 	CreateUUID    = func() string { return uuid.New().String() }
 	isStdinATTY   = term.IsTerminal(int(os.Stdin.Fd()))
+	isStdoutATTY  = term.IsTerminal(int(os.Stdout.Fd()))
 	anonymousUser = telemetry.User{
 		UniqueID: "cli-anonymous-telemetry",
 	}
@@ -125,8 +126,9 @@ func loadTelemetrySettings(settings *settings.TelemetrySettings, user *telemetry
 		return
 	}
 
-	// If stdin is not available, send telemetry event, disable telemetry and return
-	if !isStdinATTY {
+	// If stdin is not a TTY, stdout is not a TTY or we are in a CI environment, send telemetry event, disable telemetry and return
+	isDisabledByDefault := !isStdinATTY || !isStdoutATTY || os.Getenv("CI") == trueString
+	if isDisabledByDefault {
 		settings.IsEnabled = false
 		err := telemetry.SendTelemetryApproval(anonymousUser, telemetry.NoStdin)
 		if err != nil {

cmd/create_telemetry_test.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -10,64 +11,12 @@ import (
 	"gotest.tools/v3/assert"
 )
 
-type testTelemetry struct {
-	events []telemetry.Event
-	User   telemetry.User
-}
-
-func (cli *testTelemetry) Close() error { return nil }
-
-func (cli *testTelemetry) Track(event telemetry.Event) error {
-	newEvent := event
-	properties := map[string]interface{}{}
-	if cli.User.UniqueID != "" {
-		properties["UUID"] = cli.User.UniqueID
-	}
-
-	if cli.User.UserID != "" {
-		properties["user_id"] = cli.User.UserID
-	}
-
-	properties["is_self_hosted"] = cli.User.IsSelfHosted
-
-	if cli.User.OS != "" {
-		properties["os"] = cli.User.OS
-	}
-
-	if cli.User.Version != "" {
-		properties["cli_version"] = cli.User.Version
-	}
-
-	if cli.User.TeamName != "" {
-		properties["team_name"] = cli.User.TeamName
-	}
-
-	if len(properties) > 0 {
-		newEvent.Properties = properties
-	}
-
-	cli.events = append(cli.events, newEvent)
-	return nil
-}
-
-type telemetryTestUI struct {
-	Approved bool
-}
-
-func (ui telemetryTestUI) AskUserToApproveTelemetry(message string) bool {
-	return ui.Approved
-}
-
-type telemetryTestAPIClient struct {
-	id  string
-	err error
-}
-
-func (me telemetryTestAPIClient) GetMyUserId() (string, error) {
-	return me.id, me.err
-}
-
 func TestLoadTelemetrySettings(t *testing.T) {
+	// Delete env.CI variable
+	oldEnvCIValue := os.Getenv("CI")
+	os.Setenv("CI", "")
+	defer os.Setenv("CI", oldEnvCIValue)
+
 	// Mock HTTP
 	userId := "id"
 	uniqueId := "unique-id"
@@ -80,6 +29,8 @@ func TestLoadTelemetrySettings(t *testing.T) {
 	// Create test cases
 	type args struct {
 		closeStdin     bool
+		closeStdout    bool
+		env            map[string]string
 		promptApproval bool
 		settings       settings.TelemetrySettings
 	}
@@ -172,12 +123,11 @@ func TestLoadTelemetrySettings(t *testing.T) {
 				settings: settings.TelemetrySettings{
 					HasAnsweredPrompt: true,
 				},
-				fileNotCreated:  true,
-				telemetryEvents: []telemetry.Event{},
+				fileNotCreated: true,
 			},
 		},
 		{
-			name: "Does not change telemetry settings if stdin is not open",
+			name: "Does not change telemetry settings if stdin is not a TTY",
 			args: args{closeStdin: true},
 			want: want{
 				fileNotCreated: true,
@@ -189,6 +139,65 @@ func TestLoadTelemetrySettings(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "Does not change telemetry settings if stdout is not a TTY",
+			args: args{closeStdout: true},
+			want: want{
+				fileNotCreated: true,
+				telemetryEvents: []telemetry.Event{
+					{Object: "cli-telemetry", Action: "disabled_default", Properties: map[string]interface{}{
+						"UUID":           "cli-anonymous-telemetry",
+						"is_self_hosted": false,
+					}},
+				},
+			},
+		},
+		{
+			name: "Does not change telemetry settings if env.CI == true",
+			args: args{env: map[string]string{"CI": "true"}},
+			want: want{
+				fileNotCreated: true,
+				telemetryEvents: []telemetry.Event{
+					{Object: "cli-telemetry", Action: "disabled_default", Properties: map[string]interface{}{
+						"UUID":           "cli-anonymous-telemetry",
+						"is_self_hosted": false,
+					}},
+				},
+			},
+		},
+		{
+			name: "Should try loading user id if user already answered prompt but has no user id",
+			args: args{
+				settings: settings.TelemetrySettings{
+					HasAnsweredPrompt: true,
+					IsEnabled:         true,
+				},
+			},
+			want: want{
+				settings: settings.TelemetrySettings{
+					HasAnsweredPrompt: true,
+					IsEnabled:         true,
+					UserID:            userId,
+				},
+			},
+		},
+		{
+			name: "Should consider has_answered_prompt before disabled-default scenarios",
+			args: args{
+				env: map[string]string{"CI": "true"},
+				settings: settings.TelemetrySettings{
+					HasAnsweredPrompt: true,
+					IsEnabled:         true,
+				},
+			},
+			want: want{
+				settings: settings.TelemetrySettings{
+					HasAnsweredPrompt: true,
+					IsEnabled:         true,
+					UserID:            userId,
+				},
+			},
+		},
 	}
 
 	for _, tt := range testCases {
@@ -203,6 +212,11 @@ func TestLoadTelemetrySettings(t *testing.T) {
 			isStdinATTY = !tt.args.closeStdin
 			defer (func() { isStdinATTY = oldIsStdinOpen })()
 
+			// Mock stdout
+			oldIsStdoutOpen := isStdoutATTY
+			isStdoutATTY = !tt.args.closeStdout
+			defer (func() { isStdoutATTY = oldIsStdoutOpen })()
+
 			// Mock telemetry
 			telemetryClient := testTelemetry{events: make([]telemetry.Event, 0)}
 			oldCreateActiveTelemetry := telemetry.CreateActiveTelemetry
@@ -212,12 +226,25 @@ func TestLoadTelemetrySettings(t *testing.T) {
 			}
 			defer (func() { telemetry.CreateActiveTelemetry = oldCreateActiveTelemetry })()
 
+			// Mock env
+			if tt.args.env != nil {
+				for k, v := range tt.args.env {
+					oldEnvValue := os.Getenv(k)
+					os.Setenv(k, v)
+					defer os.Setenv(k, oldEnvValue)
+				}
+			}
+
 			// Run tested function
 			loadTelemetrySettings(&tt.args.settings, &telemetry.User{}, telemetryTestAPIClient{userId, nil}, telemetryTestUI{tt.args.promptApproval})
 			assert.DeepEqual(t, &tt.args.settings, &tt.want.settings)
 
 			// Verify good telemetry events were sent
-			assert.DeepEqual(t, telemetryClient.events, tt.want.telemetryEvents)
+			expectedEvents := tt.want.telemetryEvents
+			if expectedEvents == nil {
+				expectedEvents = []telemetry.Event{}
+			}
+			assert.DeepEqual(t, telemetryClient.events, expectedEvents)
 
 			// Verify if settings file exist
 			exist, err := settings.FS.Exists(filepath.Join(settings.SettingsPath(), "telemetry.yml"))
@@ -235,3 +262,60 @@ func TestLoadTelemetrySettings(t *testing.T) {
 		})
 	}
 }
+
+type testTelemetry struct {
+	events []telemetry.Event
+	User   telemetry.User
+}
+
+func (cli *testTelemetry) Close() error { return nil }
+
+func (cli *testTelemetry) Track(event telemetry.Event) error {
+	newEvent := event
+	properties := map[string]interface{}{}
+	if cli.User.UniqueID != "" {
+		properties["UUID"] = cli.User.UniqueID
+	}
+
+	if cli.User.UserID != "" {
+		properties["user_id"] = cli.User.UserID
+	}
+
+	properties["is_self_hosted"] = cli.User.IsSelfHosted
+
+	if cli.User.OS != "" {
+		properties["os"] = cli.User.OS
+	}
+
+	if cli.User.Version != "" {
+		properties["cli_version"] = cli.User.Version
+	}
+
+	if cli.User.TeamName != "" {
+		properties["team_name"] = cli.User.TeamName
+	}
+
+	if len(properties) > 0 {
+		newEvent.Properties = properties
+	}
+
+	cli.events = append(cli.events, newEvent)
+	return nil
+}
+
+type telemetryTestUI struct {
+	Approved bool
+}
+
+func (ui telemetryTestUI) AskUserToApproveTelemetry(message string) bool {
+	return ui.Approved
+}
+
+type telemetryTestAPIClient struct {
+	id  string
+	err error
+}
+
+func (me telemetryTestAPIClient) GetMyUserId() (string, error) {
+	return me.id, me.err
+}

settings/settings.go

@@ -28,21 +28,22 @@ var (
 
 // Config is used to represent the current state of a CLI instance.
 type Config struct {
-	Host                string        `yaml:"host"`
-	DlHost              string        `yaml:"-"`
-	Endpoint            string        `yaml:"endpoint"`
-	Token               string        `yaml:"token"`
-	RestEndpoint        string        `yaml:"rest_endpoint"`
-	TLSCert             string        `yaml:"tls_cert"`
-	TLSInsecure         bool          `yaml:"tls_insecure"`
-	HTTPClient          *http.Client  `yaml:"-"`
-	Data                *data.DataBag `yaml:"-"`
-	Debug               bool          `yaml:"-"`
-	Address             string        `yaml:"-"`
-	FileUsed            string        `yaml:"-"`
-	GitHubAPI           string        `yaml:"-"`
-	SkipUpdateCheck     bool          `yaml:"-"`
-	IsTelemetryDisabled bool          `yaml:"-"`
+	Host            string        `yaml:"host"`
+	DlHost          string        `yaml:"-"`
+	Endpoint        string        `yaml:"endpoint"`
+	Token           string        `yaml:"token"`
+	RestEndpoint    string        `yaml:"rest_endpoint"`
+	TLSCert         string        `yaml:"tls_cert"`
+	TLSInsecure     bool          `yaml:"tls_insecure"`
+	HTTPClient      *http.Client  `yaml:"-"`
+	Data            *data.DataBag `yaml:"-"`
+	Debug           bool          `yaml:"-"`
+	Address         string        `yaml:"-"`
+	FileUsed        string        `yaml:"-"`
+	GitHubAPI       string        `yaml:"-"`
+	SkipUpdateCheck bool          `yaml:"-"`
+	// Parameter used to disable telemetry from tests
+	IsTelemetryDisabled bool `yaml:"-"`
 	// If this value is defined, the telemetry will write all its events a file
 	// The value of this field is the path where the telemetry will be written
 	MockTelemetry string            `yaml:"-"`