Resolve conflicting environment variables among test targets in custom schemes (#3284)

Fixes #3283 

This takes a similar approach to how rules_apple handles infoplist
merging (take the union of top-level key/values, throw an error if a key
matches but a value does not).

Signed-off-by: Aaron Sky <aaronsky@skyaaron.com>

Author: aaronsky

tools/generators/lib/XCScheme/src/EnvironmentVariables.swift

@@ -1,7 +1,7 @@
 public struct EnvironmentVariable: Equatable {
-    let key: String
-    let value: String
-    let isEnabled: Bool
+    public let key: String
+    public let value: String
+    public let isEnabled: Bool
 
     public init(key: String, value: String, isEnabled: Bool = true) {
         self.key = key

tools/generators/xcschemes/BUILD

@@ -79,6 +79,8 @@ macos_unit_test(
     ],
     deps = [
         ":xcschemes_tests.library",
+        "//tools/generators/lib/PBXProj",
+        "//tools/generators/lib/XCScheme",
     ],
 )
 

tools/generators/xcschemes/src/Generator/CreateCustomSchemeInfos.swift

@@ -702,27 +702,8 @@ set
         var environmentVariables: [EnvironmentVariable]
         if let specifiedEnvironmentVariables {
             environmentVariables = specifiedEnvironmentVariables
-        } else if let firstTestTargetID, let aEnvironmentVariables =
-            targetEnvironmentVariables[firstTestTargetID]
-        {
-            // If the custom scheme inherits environment variables, and every
-            // test target defines the same env, then use them
-            var allEnvironmentVariablesTheSame = true
-            for testTarget in testTargets {
-                let id = testTarget.target.key.sortedIds.first!
-                guard aEnvironmentVariables ==
-                    targetEnvironmentVariables[id]
-                else {
-                    allEnvironmentVariablesTheSame = false
-                    break
-                }
-            }
-
-            if allEnvironmentVariablesTheSame {
-                environmentVariables = aEnvironmentVariables
-            } else {
-                environmentVariables = []
-            }
+        } else if let firstTestTargetID, targetEnvironmentVariables[firstTestTargetID] != nil {
+            environmentVariables = try mergingEnvironmentVariables(targetEnvironmentVariables, in: testTargets)
         } else {
             environmentVariables = []
         }
@@ -753,6 +734,31 @@ set
     }
 }
 
+func mergingEnvironmentVariables(
+    _ targetEnvironmentVariables: [TargetID: [EnvironmentVariable]],
+    in testTargets: [SchemeInfo.TestTarget]
+) throws -> [EnvironmentVariable] {
+    var uniqueEnvironmentVariables: [String: EnvironmentVariable] = [:]
+
+    for testTarget in testTargets {
+        let id = testTarget.target.key.sortedIds.first!
+
+        for variable in targetEnvironmentVariables[id, default: []] {
+            if let storedVariable = uniqueEnvironmentVariables[variable.key],
+                storedVariable.value != variable.value {
+                throw UsageError(message: """
+(\(id)) found key "\(variable.key)" in two environment variable \
+declarations with different values: "\(storedVariable.value)" != "\(variable.value)"
+""")
+            }
+
+            uniqueEnvironmentVariables[variable.key] = variable
+        }
+    }
+
+    return uniqueEnvironmentVariables.values.sorted { $0.key < $1.key }
+}
+
 private extension Dictionary where
     Key == String, Value == [SchemeInfo.ExecutionAction]
 {

tools/generators/xcschemes/test/CreateCustomSchemeInfosTests.swift

@@ -1,8 +1,82 @@
+import PBXProj
+import XCScheme
 import XCTest
 
 @testable import xcschemes
 
 final class CreateCustomSchemeInfosTests: XCTestCase {
+    func test_merging_environment_variables() throws {
+        let targets: [SchemeInfo.TestTarget] = [
+            .init(
+                target: .init(
+                    key: .init([.init("target1")]),
+                    productType: .unitTestBundle,
+                    buildableReference: .init(
+                        blueprintIdentifier: "",
+                        buildableName: "",
+                        blueprintName: "",
+                        referencedContainer: "",
+                    )
+                ),
+                isEnabled: true
+            ),
+            .init(
+                target: .init(
+                    key: .init([.init("target2")]),
+                    productType: .unitTestBundle,
+                    buildableReference: .init(
+                        blueprintIdentifier: "",
+                        buildableName: "",
+                        blueprintName: "",
+                        referencedContainer: "",
+                    )
+                ),
+                isEnabled: true
+            ),
+        ]
+
+        // No environment variables
+        try XCTAssert(mergingEnvironmentVariables([:], in: []).isEmpty)
+
+        // Environment variables with no overlap
+        try XCTAssertEqual(
+            mergingEnvironmentVariables(
+                [
+                    "target1": [EnvironmentVariable(key: "VAR1", value: "value1", isEnabled: true)],
+                    "target2": [EnvironmentVariable(key: "VAR2", value: "value2", isEnabled: true)],
+                ],
+                in: targets
+            ),
+            [
+                EnvironmentVariable(key: "VAR1", value: "value1", isEnabled: true),
+                EnvironmentVariable(key: "VAR2", value: "value2", isEnabled: true),
+            ]
+        )
+
+        // Environment variables with overlap (target1 and target2 both have VAR1, and the output should contain VAR1 because the values match.
+        try XCTAssertEqual(
+            mergingEnvironmentVariables(
+                [
+                    "target1": [EnvironmentVariable(key: "VAR1", value: "value1", isEnabled: true)],
+                    "target2": [EnvironmentVariable(key: "VAR1", value: "value1", isEnabled: true)],
+                ],
+                in: targets
+            ),
+            [EnvironmentVariable(key: "VAR1", value: "value1", isEnabled: true)]
+        )
+
+        // Environment variables with overlap but different values (target1 and target2 both have VAR1, but the values differ, so the output should be empty because there is no consistent value for VAR1).
+        try XCTAssertThrowsError(
+            mergingEnvironmentVariables(
+                [
+                    "target1": [EnvironmentVariable(key: "VAR1", value: "value1", isEnabled: true)],
+                    "target2": [EnvironmentVariable(key: "VAR1", value: "value2", isEnabled: true)],
+                ],
+                in: targets
+            )
+        )
+    }
+
     func test_url_relativize() {
         typealias TestCase = (dest: URL, source: URL, expected: String?)
         let testCases: [TestCase] = [