From f8757b6ee710734f0f684fa502a76991ccb9e7d4 Mon Sep 17 00:00:00 2001
From: Bartosz Polaczyk <polac24@gmail.com>
Date: Mon, 29 May 2023 21:01:53 -0700
Subject: [PATCH] Add unit tests

---
 .../Swiftc/NoopSwiftcProductsGenerator.swift  | 38 +++++++++++++++++
 .../Commands/Swiftc/SwiftcContext.swift       | 10 ++---
 .../Swiftc/SwiftcFilemapInputEditor.swift     |  8 +++-
 .../Commands/Swiftc/XCSwiftc.swift            |  9 +++-
 .../Commands/SwiftcContextTests.swift         | 38 +++++++++++++++--
 .../Commands/SwiftcOrchestratorTests.swift    | 41 +++++++++++++++++++
 6 files changed, 132 insertions(+), 12 deletions(-)
 create mode 100644 Sources/XCRemoteCache/Commands/Swiftc/NoopSwiftcProductsGenerator.swift

diff --git a/Sources/XCRemoteCache/Commands/Swiftc/NoopSwiftcProductsGenerator.swift b/Sources/XCRemoteCache/Commands/Swiftc/NoopSwiftcProductsGenerator.swift
new file mode 100644
index 00000000..a1a4e7de
--- /dev/null
+++ b/Sources/XCRemoteCache/Commands/Swiftc/NoopSwiftcProductsGenerator.swift
@@ -0,0 +1,38 @@
+// Copyright (c) 2021 Spotify AB.
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+import Foundation
+
+/// Products generator that doesn't create any swiftmodule. It is used in the compilation swift-frontend mocking, where
+/// only individual .o files are created and not .swiftmodule of -Swift.h
+/// (which is part of swift-frontend -emit-module invocation)
+class NoopSwiftcProductsGenerator: SwiftcProductsGenerator {
+    func generateFrom(
+        artifactSwiftModuleFiles: [SwiftmoduleFileExtension: URL],
+        artifactSwiftModuleObjCFile: URL
+    ) throws -> SwiftcProductsGeneratorOutput {
+        infoLog("""
+        Invoking module generation from NoopSwiftcProductsGenerator does nothing. \
+        It might be a side-effect of a plugin asking to generate a module.
+        """)
+        // NoopSwiftcProductsGenerator is intended only for the swift-frontend
+        let trivialURL = URL(fileURLWithPath: "/non-existing")
+        return SwiftcProductsGeneratorOutput(swiftmoduleDir: trivialURL, objcHeaderFile: trivialURL)
+    }
+}
diff --git a/Sources/XCRemoteCache/Commands/Swiftc/SwiftcContext.swift b/Sources/XCRemoteCache/Commands/Swiftc/SwiftcContext.swift
index a99757ae..485c9d18 100644
--- a/Sources/XCRemoteCache/Commands/Swiftc/SwiftcContext.swift
+++ b/Sources/XCRemoteCache/Commands/Swiftc/SwiftcContext.swift
@@ -22,7 +22,7 @@ import Foundation
 public struct SwiftcContext {
     /// Describes the action if the module emit should happen
     /// that generates .swiftmodule and/or -Swift.h
-    public struct SwiftcStepEmitModule {
+    public struct SwiftcStepEmitModule: Equatable {
         // where the -Swift.h should be placed
         let objcHeaderOutput: URL
         // where should the .swiftmodule be placed
@@ -34,7 +34,7 @@ public struct SwiftcContext {
 
     /// Which files (from the list of all files in the module)
     /// should be compiled in this process
-    public enum SwiftcStepCompileFilesScope {
+    public enum SwiftcStepCompileFilesScope: Equatable {
         /// used if only emit module should be done
         case none
         case all
@@ -42,7 +42,7 @@ public struct SwiftcContext {
     }
 
     /// Describes which steps should be done as a part of this process
-    public struct SwiftcSteps {
+    public struct SwiftcSteps: Equatable {
         /// which files should be compiled
         let compileFilesScope: SwiftcStepCompileFilesScope
         /// if a module should be generated
@@ -50,7 +50,7 @@ public struct SwiftcContext {
     }
 
     /// Defines how a list of input files (*.swift) is passed to the invocation
-    public enum CompilationFilesSource {
+    public enum CompilationFilesSource: Equatable {
         /// defined in a separate file (via @/.../*.SwiftFileList)
         case fileList(String)
         /// explicitly passed a list of files
@@ -58,7 +58,7 @@ public struct SwiftcContext {
     }
 
     /// Defines how a list of output files (*.d, *.o etc.) is passed to the invocation
-    public enum CompilationFilesInputs {
+    public enum CompilationFilesInputs: Equatable {
         /// defined in a separate file (via -output-file-map)
         case fileMap(String)
         /// defined in a separate file (via -supplementary-output-file-map)
diff --git a/Sources/XCRemoteCache/Commands/Swiftc/SwiftcFilemapInputEditor.swift b/Sources/XCRemoteCache/Commands/Swiftc/SwiftcFilemapInputEditor.swift
index 2a1bedf9..6256d18e 100644
--- a/Sources/XCRemoteCache/Commands/Swiftc/SwiftcFilemapInputEditor.swift
+++ b/Sources/XCRemoteCache/Commands/Swiftc/SwiftcFilemapInputEditor.swift
@@ -24,7 +24,10 @@ import Yams
 enum SwiftcInputReaderError: Error {
     case readingFailed
     case invalidFormat
+    /// The file is not in the yaml format
     case invalidYamlFormat
+    /// The yaml string contains illegal characters
+    case invalidYamlString
     case missingField(String)
 }
 
@@ -98,7 +101,10 @@ class SwiftcFilemapInputEditor: SwiftcInputReader, SwiftcInputWriter {
         case .json:
             return try JSONSerialization.jsonObject(with: content, options: []) as? [String: Any]
         case .yaml:
-            return try Yams.load(yaml: String(data: content, encoding: .utf8)!) as? [String: Any]
+            guard let stringContent = String(data: content, encoding: .utf8) else {
+                throw SwiftcInputReaderError.invalidYamlString
+            }
+            return try Yams.load(yaml: stringContent) as? [String: Any]
         }
     }
 }
diff --git a/Sources/XCRemoteCache/Commands/Swiftc/XCSwiftc.swift b/Sources/XCRemoteCache/Commands/Swiftc/XCSwiftc.swift
index b35febf3..ee40becc 100644
--- a/Sources/XCRemoteCache/Commands/Swiftc/XCSwiftc.swift
+++ b/Sources/XCRemoteCache/Commands/Swiftc/XCSwiftc.swift
@@ -86,6 +86,8 @@ public class XCSwiftAbstract<InputArgs> {
                 fileManager: fileManager
             )
         case .supplementaryFileMap(let path):
+            // Supplementary file map is endoded in the yaml file (contraty to
+            // the standard filemap, which is in json)
             inputReader = SwiftcFilemapInputEditor(
                 URL(fileURLWithPath: path),
                 fileFormat: .yaml,
@@ -93,10 +95,9 @@ public class XCSwiftAbstract<InputArgs> {
             )
         case .map(let map):
             // static - passed via the arguments list
-            // TODO: check if first 2 ars can always be `nil`
-            // with Xcode 14, inputs via cmd are only used for compilations
             inputReader = StaticSwiftcInputReader(
                 moduleDependencies: context.steps.emitModule?.dependencies,
+                // with Xcode 14, inputs via cmd are only used for compilations
                 swiftDependencies: nil,
                 compilationFiles: Array(map.values)
             )
@@ -134,6 +135,10 @@ public class XCSwiftAbstract<InputArgs> {
                 diskCopier: HardLinkDiskCopier(fileManager: fileManager)
             )
         } else {
+            // If the module was not requested for this proces (compiling files only)
+            // do nothing, when someone (e.g. a plugin) asks for the products generation
+            // This generation will happend in a separate process, where the module
+            // generation is requested
             productsGenerator = NoopSwiftcProductsGenerator()
         }
         let allInvocationsStorage = ExistingFileStorage(
diff --git a/Tests/XCRemoteCacheTests/Commands/SwiftcContextTests.swift b/Tests/XCRemoteCacheTests/Commands/SwiftcContextTests.swift
index 30020e11..8fc08c4d 100644
--- a/Tests/XCRemoteCacheTests/Commands/SwiftcContextTests.swift
+++ b/Tests/XCRemoteCacheTests/Commands/SwiftcContextTests.swift
@@ -25,20 +25,25 @@ class SwiftcContextTests: FileXCTestCase {
     private var config: XCRemoteCacheConfig!
     private var input: SwiftcArgInput!
     private var remoteCommitFile: URL!
+    private var modulePathOutput: URL!
+    private var fileMapUrl: URL!
+    private var fileListUrl: URL!
 
     override func setUpWithError() throws {
         try super.setUpWithError()
         let workingDir = try prepareTempDir()
         remoteCommitFile = workingDir.appendingPathComponent("arc.rc")
-        let modulePathOutput = workingDir.appendingPathComponent("mpo")
+        modulePathOutput = workingDir.appendingPathComponent("mpo")
+        fileMapUrl = workingDir.appendingPathComponent("filemap")
+        fileListUrl = workingDir.appendingPathComponent("filelist")
         config = XCRemoteCacheConfig(remoteCommitFile: remoteCommitFile.path, sourceRoot: workingDir.path)
         input = SwiftcArgInput(
             objcHeaderOutput: "Target-Swift.h",
-            moduleName: "Target",
+            moduleName: "Module",
             modulePathOutput: modulePathOutput.path,
-            filemap: "",
+            filemap: fileMapUrl.path,
             target: "",
-            fileList: ""
+            fileList: fileListUrl.path
         )
         try fileManager.write(toPath: remoteCommitFile.path, contents: "123".data(using: .utf8))
     }
@@ -77,4 +82,29 @@ class SwiftcContextTests: FileXCTestCase {
 
         XCTAssertEqual(context.mode, .producer)
     }
+
+    func testStepsContainEmitingModuleAndAllCompilationScope() throws {
+        let context = try SwiftcContext(config: config, input: input)
+
+        XCTAssertEqual(context.steps, .init(
+            compileFilesScope: .all,
+            emitModule: .init(
+                objcHeaderOutput: "Target-Swift.h",
+                modulePathOutput: modulePathOutput,
+                dependencies: nil)
+            )
+        )
+    }
+
+    func testReadsInputsFromFileMap() throws {
+        let context = try SwiftcContext(config: config, input: input)
+
+        XCTAssertEqual(context.inputs, .fileMap(fileMapUrl.path))
+    }
+
+    func testReadsCompilationFilesFromFileList() throws {
+        let context = try SwiftcContext(config: config, input: input)
+
+        XCTAssertEqual(context.compilationFiles, .fileList(fileListUrl.path))
+    }
 }
diff --git a/Tests/XCRemoteCacheTests/Commands/SwiftcOrchestratorTests.swift b/Tests/XCRemoteCacheTests/Commands/SwiftcOrchestratorTests.swift
index 67bb1686..4d93cb4f 100644
--- a/Tests/XCRemoteCacheTests/Commands/SwiftcOrchestratorTests.swift
+++ b/Tests/XCRemoteCacheTests/Commands/SwiftcOrchestratorTests.swift
@@ -231,4 +231,45 @@ class SwiftcOrchestratorTests: XCTestCase {
         XCTAssertEqual(artifactBuilder.addedObjCHeaders, [:])
     }
 
+
+    func testNotSetObjCHeaderIsNotCreated() throws {
+        let swiftc = SwiftcMock(mockingResult: .success)
+        let orchestrator = SwiftcOrchestrator(
+            mode: .producer,
+            swiftc: swiftc,
+            swiftcCommand: "",
+            objcHeaderOutput: nil,
+            moduleOutput: moduleOutputURL,
+            arch: "arch",
+            artifactBuilder: artifactBuilder,
+            producerFallbackCommandProcessors: [],
+            invocationStorage: invocationStorage,
+            shellOut: shellOutSpy
+        )
+
+        try orchestrator.run()
+
+        XCTAssertEqual(artifactBuilder.addedObjCHeaders, [:])
+    }
+
+    func testNotSetModuleOutputIsNotCreated() throws {
+        let swiftc = SwiftcMock(mockingResult: .success)
+        let orchestrator = SwiftcOrchestrator(
+            mode: .producer,
+            swiftc: swiftc,
+            swiftcCommand: "",
+            objcHeaderOutput: objcHeaderURL,
+            moduleOutput: nil,
+            arch: "arch",
+            artifactBuilder: artifactBuilder,
+            producerFallbackCommandProcessors: [],
+            invocationStorage: invocationStorage,
+            shellOut: shellOutSpy
+        )
+
+        try orchestrator.run()
+
+        XCTAssertEqual(artifactBuilder.addedModuleDefinitions, [:])
+    }
+
 }