fix: stop xcodebuild collecting a sysdiagnose the user did not ask for

README.md

@@ -109,7 +109,7 @@ Run tests with custom xcconfig file path:
 | `xcpretty_options` | Additional options to be added to the executed xcpretty command. |  | `--color --report html --output "${BITRISE_DEPLOY_DIR}/xcode-test-results-${BITRISE_SCHEME}.html"` |
 | `cache_level` | Defines what cache content should be automatically collected. Use key-based caching instead for better performance.  Available options: - `none`: Disable collecting cache content. - `swift_packages`: Collect Swift PM packages added to the Xcode project.  With key-based caching, you only need the Restore SPM cache and the Save SPM cache Steps to cache your Swift packages. [See devcenter for more information.](https://devcenter.bitrise.io/en/dependencies-and-caching/managing-dependencies-for-ios-apps/managing-dependencies-with-spm.html#caching-swift-packages) |  | `none` |
 | `verbose_log` | If this input is set, the Step will print additional logs for debugging. |  | `no` |
-| `collect_simulator_diagnostics` | If this input is set, the simulator verbose logging will be enabled and the simulator diagnostics log will be exported. |  | `never` |
+| `collect_simulator_diagnostics` | Enables Simulator verbose logging and controls whether Simulator diagnostics are collected after the test run.  xcodebuild collects the Simulator diagnostics (a `simctl diagnose` bundle) itself after a failing test run and stores them in the `.xcresult`; extract them with `xcrun xcresulttool export diagnostics`. Simulator verbose logging is enabled before the tests run unless this Input is `never`.  The default value is `never`, which prevents intermittent hangs in a CI environment. To let xcodebuild follow the test plan (collect on failure unless the plan says otherwise), set this Input to `project_setting`. A `-collect-test-diagnostics` option passed in `xcodebuild_options` takes precedence over this Input.  On Xcode 16 and earlier, xcodebuild has no such option and the Step collects the diagnostics itself after the tests and exports them as a zip artifact.  Available options: - `project_setting`: use the option set in the Xcode project test plan. - `always`: same as `on_failure` on Xcode 26 and later, where xcodebuild collects the diagnostics itself. On Xcode 16 and earlier the Step also collects after passing runs. - `on_failure`: collect diagnostics when the tests fail. - `never`: overrides any test plan setting and prevents the collection of diagnostics. |  | `never` |
 | `headless_mode` | In headless mode the simulator is not launched in the foreground.  If this input is set, the simulator will not be visible but tests (even the screenshots) will run just like if you run a simulator in foreground. |  | `yes` |
 | `quarantined_tests` | JSON list of tests added to quarantine on Bitrise.io, quarantined tests are excluded from test runs. |  | `$BITRISE_QUARANTINED_TESTS_JSON` |
 </details>

bitrise.yml

@@ -1,14 +1,17 @@
 format_version: "11"
 default_step_lib_source: https://github.com/bitrise-io/bitrise-steplib.git
 
+include:
+- repository: steps-check
+  branch: master
+  path: steps.bitrise.yml
+
 app:
   envs:
   - ORIG_BITRISE_SOURCE_DIR: $BITRISE_SOURCE_DIR
 
 workflows:
-  check:
-    steps:
-    - git::https://github.com/bitrise-steplib/steps-check.git: { }
+  # check: included from steps-check/steps.bitrise.yml
 
   e2e:
     steps:

e2e/bitrise.yml

@@ -209,7 +209,7 @@ workflows:
         - repository_url: $TEST_APP_URL
         - clone_into_dir: ./_tmp
         - branch: $TEST_APP_BRANCH
-    - certificate-and-profile-installer: { }
+    - certificate-and-profile-installer: {}
     - script:
         title: Set 'collect_simulator_diagnostics' input
         inputs:

main.go

@@ -38,13 +38,13 @@ func run() int {
 	configParser := createConfigParser(logger)
 	config, err := configParser.ProcessConfig()
 	if err != nil {
-		logger.Errorf(errorutil.FormattedError(fmt.Errorf("Failed to process Step inputs: %w", err)))
+		logger.Errorf("%s", errorutil.FormattedError(fmt.Errorf("Failed to process Step inputs: %w", err))) //nolint:staticcheck // top-level messages are user facing
 		return 1
 	}
 
 	xcodeTestRunner, err := createStep(logger, config.LogFormatter)
 	if err != nil {
-		logger.Errorf(errorutil.FormattedError(fmt.Errorf("Failed to process Step inputs: %w", err)))
+		logger.Errorf("%s", errorutil.FormattedError(fmt.Errorf("Failed to process Step inputs: %w", err))) //nolint:staticcheck // top-level messages are user facing
 		return 1
 	}
 
@@ -54,12 +54,12 @@ func run() int {
 	exportErr := xcodeTestRunner.Export(res, runErr != nil)
 
 	if runErr != nil {
-		logger.Errorf(errorutil.FormattedError(fmt.Errorf("Failed to execute Step: %w", runErr)))
+		logger.Errorf("%s", errorutil.FormattedError(fmt.Errorf("Failed to execute Step: %w", runErr))) //nolint:staticcheck // top-level messages are user facing
 		return 1
 	}
 
 	if exportErr != nil {
-		logger.Errorf(errorutil.FormattedError(fmt.Errorf("Failed to export Step outputs: %w", exportErr)))
+		logger.Errorf("%s", errorutil.FormattedError(fmt.Errorf("Failed to export Step outputs: %w", exportErr))) //nolint:staticcheck // top-level messages are user facing
 		return 1
 	}
 
@@ -73,13 +73,13 @@ func createConfigParser(logger log.Logger) step.XcodeTestConfigParser {
 	xcodeVersionProvider := xcodeversion.NewXcodeVersionProvider(commandFactory)
 	xcodeVersion, err := xcodeVersionProvider.GetVersion()
 	if err != nil { // Not a fatal error, continuing with empty version
-		logger.Errorf("failed to read Xcode version: %w", err)
+		logger.Errorf("Failed to read Xcode version: %s", err)
 	}
 	pathModifier := pathutil.NewPathModifier()
 	deviceFinder := destination.NewDeviceFinder(logger, commandFactory, xcodeVersion)
 	utils := step.NewUtils(logger)
 
-	return step.NewXcodeTestConfigParser(inputParser, logger, deviceFinder, pathModifier, utils)
+	return step.NewXcodeTestConfigParser(inputParser, logger, deviceFinder, pathModifier, utils, xcodeVersion)
 }
 
 func createStep(logger log.Logger, logFormatter string) (step.XcodeTestRunner, error) {

step.yml

@@ -258,8 +258,28 @@ inputs:
   opts:
     category: Debugging
     title: Collect Simulator diagnostics
-    summary: If this input is set, the simulator verbose logging will be enabled and the simulator diagnostics log will be exported.
+    summary: Enables Simulator verbose logging and controls whether Simulator diagnostics are collected after the test run.
+    description: |-
+      Enables Simulator verbose logging and controls whether Simulator diagnostics are collected after the test run.
+
+      xcodebuild collects the Simulator diagnostics (a `simctl diagnose` bundle) itself after a failing test run
+      and stores them in the `.xcresult`; extract them with `xcrun xcresulttool export diagnostics`.
+      Simulator verbose logging is enabled before the tests run unless this Input is `never`.
+
+      The default value is `never`, which prevents intermittent hangs in a CI environment.
+      To let xcodebuild follow the test plan (collect on failure unless the plan says otherwise), set this Input to `project_setting`.
+      A `-collect-test-diagnostics` option passed in `xcodebuild_options` takes precedence over this Input.
+
+      On Xcode 16 and earlier, xcodebuild has no such option and the Step collects the diagnostics itself after the
+      tests and exports them as a zip artifact.
+
+      Available options:
+      - `project_setting`: use the option set in the Xcode project test plan.
+      - `always`: same as `on_failure` on Xcode 26 and later, where xcodebuild collects the diagnostics itself. On Xcode 16 and earlier the Step also collects after passing runs.
+      - `on_failure`: collect diagnostics when the tests fail.
+      - `never`: overrides any test plan setting and prevents the collection of diagnostics.
     value_options:
+    - project_setting
     - always
     - on_failure
     - never

step/diagnostics_test.go

@@ -0,0 +1,157 @@
+package step
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func Test_xcodebuildDiagnosticsOverride(t *testing.T) {
+	tests := []struct {
+		name              string
+		condition         exportCondition
+		xcodeMajorVersion int64
+		additionalOptions []string
+		want              string
+	}{
+		{
+			name:              "Xcode 26, never - suppresses xcodebuild's own collection",
+			condition:         never,
+			xcodeMajorVersion: 26,
+			want:              "never",
+		},
+		{
+			name:              "Xcode 27, never - suppresses xcodebuild's own collection",
+			condition:         never,
+			xcodeMajorVersion: 27,
+			want:              "never",
+		},
+		{
+			name:              "Xcode 27, on_failure - keeps xcodebuild's collection",
+			condition:         onFailure,
+			xcodeMajorVersion: 27,
+			want:              "on-failure",
+		},
+		{
+			// xcodebuild has no "always"; its collection is failure-triggered regardless.
+			name:              "Xcode 27, always - maps onto on-failure",
+			condition:         always,
+			xcodeMajorVersion: 27,
+			want:              "on-failure",
+		},
+		{
+			// project_setting leaves the decision to the test plan, so nothing is passed.
+			name:              "Xcode 27, project_setting - option not passed",
+			condition:         projectSetting,
+			xcodeMajorVersion: 27,
+			want:              "",
+		},
+		{
+			// The option does not exist before Xcode 26; passing it would be a usage error.
+			name:              "Xcode 25, never - option not passed",
+			condition:         never,
+			xcodeMajorVersion: 25,
+			want:              "",
+		},
+		{
+			name:              "Xcode 15, always - option not passed",
+			condition:         always,
+			xcodeMajorVersion: 15,
+			want:              "",
+		},
+		{
+			// main.go treats an unreadable Xcode version as non-fatal and leaves Major at 0.
+			name:              "unknown Xcode version - option not passed",
+			condition:         never,
+			xcodeMajorVersion: 0,
+			want:              "",
+		},
+		{
+			name:              "user set the option explicitly - theirs wins",
+			condition:         never,
+			xcodeMajorVersion: 27,
+			additionalOptions: []string{"-collect-test-diagnostics", "on-failure"},
+			want:              "",
+		},
+		{
+			// xcodebuild silently drops the -option=value form, so it must not count as an override.
+			name:              "user wrote the option with = syntax - Step still passes its own",
+			condition:         never,
+			xcodeMajorVersion: 27,
+			additionalOptions: []string{"-collect-test-diagnostics=on-failure"},
+			want:              "never",
+		},
+		{
+			name:              "unrelated additional options are ignored",
+			condition:         never,
+			xcodeMajorVersion: 27,
+			additionalOptions: []string{"-quiet", "-parallel-testing-enabled", "NO"},
+			want:              "never",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := xcodebuildDiagnosticsOverride(tt.condition, tt.xcodeMajorVersion, tt.additionalOptions)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func Test_shouldStepCollectDiagnostics(t *testing.T) {
+	tests := []struct {
+		name              string
+		condition         exportCondition
+		testFailed        bool
+		xcodeMajorVersion int64
+		want              bool
+	}{
+		{name: "Xcode 16, always, passed", condition: always, testFailed: false, xcodeMajorVersion: 16, want: true},
+		{name: "Xcode 16, always, failed", condition: always, testFailed: true, xcodeMajorVersion: 16, want: true},
+		{name: "Xcode 16, on_failure, passed", condition: onFailure, testFailed: false, xcodeMajorVersion: 16, want: false},
+		{name: "Xcode 16, on_failure, failed", condition: onFailure, testFailed: true, xcodeMajorVersion: 16, want: true},
+		{name: "Xcode 16, never, failed", condition: never, testFailed: true, xcodeMajorVersion: 16, want: false},
+		// project_setting has nothing to follow before Xcode 26, the Step does not collect on its own.
+		{name: "Xcode 16, project_setting, failed", condition: projectSetting, testFailed: true, xcodeMajorVersion: 16, want: false},
+		{name: "Xcode 27, project_setting, failed", condition: projectSetting, testFailed: true, xcodeMajorVersion: 27, want: false},
+		// main.go leaves the major at 0 when the Xcode version cannot be read; keep collecting there.
+		{name: "unknown Xcode, on_failure, failed", condition: onFailure, testFailed: true, xcodeMajorVersion: 0, want: true},
+		// Since Xcode 26 xcodebuild collects into the xcresult itself; the Step must not collect a second copy.
+		{name: "Xcode 26, on_failure, failed", condition: onFailure, testFailed: true, xcodeMajorVersion: 26, want: false},
+		{name: "Xcode 27, always, failed", condition: always, testFailed: true, xcodeMajorVersion: 27, want: false},
+		{name: "Xcode 27, always, passed", condition: always, testFailed: false, xcodeMajorVersion: 27, want: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := shouldStepCollectDiagnostics(tt.condition, tt.testFailed, tt.xcodeMajorVersion)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func Test_xcodebuildCollectsDiagnostics(t *testing.T) {
+	tests := []struct {
+		name              string
+		condition         exportCondition
+		testFailed        bool
+		xcodeMajorVersion int64
+		want              bool
+	}{
+		{name: "Xcode 27, on_failure, failed", condition: onFailure, testFailed: true, xcodeMajorVersion: 27, want: true},
+		{name: "Xcode 27, always, failed", condition: always, testFailed: true, xcodeMajorVersion: 27, want: true},
+		{name: "Xcode 27, project_setting, failed", condition: projectSetting, testFailed: true, xcodeMajorVersion: 27, want: true},
+		{name: "Xcode 27, never, failed", condition: never, testFailed: true, xcodeMajorVersion: 27, want: false},
+		{name: "Xcode 27, on_failure, passed", condition: onFailure, testFailed: false, xcodeMajorVersion: 27, want: false},
+		// xcodebuild has no collection of its own before Xcode 26.
+		{name: "Xcode 16, on_failure, failed", condition: onFailure, testFailed: true, xcodeMajorVersion: 16, want: false},
+		{name: "Xcode 16, project_setting, failed", condition: projectSetting, testFailed: true, xcodeMajorVersion: 16, want: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := xcodebuildCollectsDiagnostics(tt.condition, tt.testFailed, tt.xcodeMajorVersion)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}

step/step.go

@@ -20,6 +20,7 @@ import (
 	"github.com/bitrise-io/go-xcode/v2/simulator"
 	cache "github.com/bitrise-io/go-xcode/v2/xcodecache"
 	"github.com/bitrise-io/go-xcode/v2/xcodecommand"
+	"github.com/bitrise-io/go-xcode/v2/xcodeversion"
 	"github.com/bitrise-steplib/steps-xcode-test/output"
 	"github.com/bitrise-steplib/steps-xcode-test/xcodebuild"
 	"github.com/kballard/go-shellquote"
@@ -54,7 +55,7 @@ type Input struct {
 	// Debugging
 	VerboseLog                  bool   `env:"verbose_log,opt[yes,no]"`
 	QuarantinedTests            string `env:"quarantined_tests"`
-	CollectSimulatorDiagnostics string `env:"collect_simulator_diagnostics,opt[always,on_failure,never]"`
+	CollectSimulatorDiagnostics string `env:"collect_simulator_diagnostics,opt[project_setting,always,on_failure,never]"`
 	HeadlessMode                bool   `env:"headless_mode,opt[yes,no]"`
 
 	// Output export
@@ -64,9 +65,10 @@ type Input struct {
 type exportCondition string
 
 const (
-	always    = "always"
-	never     = "never"
-	onFailure = "on_failure"
+	always         = "always"
+	never          = "never"
+	onFailure      = "on_failure"
+	projectSetting = "project_setting"
 )
 
 // Output tools
@@ -97,9 +99,11 @@ type Config struct {
 
 	CacheLevel string
 
-	SkipTesting                 []string
-	CollectSimulatorDiagnostics exportCondition
-	HeadlessMode                bool
+	SkipTesting                   []string
+	CollectSimulatorDiagnostics   exportCondition
+	XcodebuildDiagnosticsOverride string
+	HeadlessMode                  bool
+	XcodeMajorVersion             int64
 
 	DeployDir string
 }
@@ -110,15 +114,17 @@ type XcodeTestConfigParser struct {
 	deviceFinder destination.DeviceFinder
 	pathModifier pathutil.PathModifier
 	utils        Utils
+	xcodeVersion xcodeversion.Version
 }
 
-func NewXcodeTestConfigParser(inputParser stepconf.InputParser, logger log.Logger, deviceFinder destination.DeviceFinder, pathModifier pathutil.PathModifier, utils Utils) XcodeTestConfigParser {
+func NewXcodeTestConfigParser(inputParser stepconf.InputParser, logger log.Logger, deviceFinder destination.DeviceFinder, pathModifier pathutil.PathModifier, utils Utils, xcodeVersion xcodeversion.Version) XcodeTestConfigParser {
 	return XcodeTestConfigParser{
 		logger:       logger,
 		inputParser:  inputParser,
 		deviceFinder: deviceFinder,
 		pathModifier: pathModifier,
 		utils:        utils,
+		xcodeVersion: xcodeVersion,
 	}
 }
 
@@ -205,7 +211,7 @@ func (s XcodeTestConfigParser) ProcessConfig() (Config, error) {
 		return Config{}, fmt.Errorf("failed to process quarentined tests: %w", err)
 	}
 
-	return s.utils.CreateConfig(input, projectPath, sim, additionalOptions, additionalLogFormatterOptions, skipTesting), nil
+	return s.utils.CreateConfig(input, projectPath, sim, additionalOptions, additionalLogFormatterOptions, skipTesting, s.xcodeVersion.Major), nil
 }
 
 /*
@@ -297,7 +303,7 @@ func (s XcodeTestRunner) Run(cfg Config) (Result, error) {
 		testExitCode = code
 	}
 
-	result.SimulatorDiagnosticsPath = s.teardownSimulator(cfg.Simulator.UDID, cfg.CollectSimulatorDiagnostics, cfg.IsSimulatorBooted, testErr)
+	result.SimulatorDiagnosticsPath = s.teardownSimulator(cfg.Simulator.UDID, cfg.CollectSimulatorDiagnostics, cfg.IsSimulatorBooted, testErr, cfg.XcodeMajorVersion)
 
 	if testErr != nil {
 		s.logger.Println()
@@ -485,10 +491,10 @@ func (s XcodeTestRunner) runTests(cfg Config) (Result, int, error) {
 	return result, exitCode, testErr
 }
 
-func (s XcodeTestRunner) teardownSimulator(simulatorID string, simulatorDebug exportCondition, isSimulatorBooted bool, testErr error) string {
+func (s XcodeTestRunner) teardownSimulator(simulatorID string, simulatorDebug exportCondition, isSimulatorBooted bool, testErr error, xcodeMajorVersion int64) string {
 	var simulatorDiagnosticsPath string
 
-	if simulatorDebug == always || (simulatorDebug == onFailure && testErr != nil) {
+	if shouldStepCollectDiagnostics(simulatorDebug, testErr != nil, xcodeMajorVersion) {
 		s.logger.Println()
 		s.logger.Infof("Collecting Simulator diagnostics")
 
@@ -499,6 +505,9 @@ func (s XcodeTestRunner) teardownSimulator(simulatorID string, simulatorDebug ex
 			s.logger.Donef("Simulator diagnostics are available as an artifact (%s)", diagnosticsPath)
 			simulatorDiagnosticsPath = diagnosticsPath
 		}
+	} else if xcodebuildCollectsDiagnostics(simulatorDebug, testErr != nil, xcodeMajorVersion) {
+		s.logger.Println()
+		s.logger.Infof("xcodebuild collects the Simulator diagnostics into the xcresult (xcrun xcresulttool export diagnostics), the Step does not collect them separately")
 	}
 
 	// Shut down the simulator if it was started by the step for diagnostic logs.