Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
[PackageToJS] Fix flaky timestampBasedRebuild test by abstracting f…
…ile system operations

The mtime-based change detection is not very reliable in tests due to
the coarse timestamp resolution of some file systems, especially on
artificial file operations in tests that happen in quick succession.
This change introduces an abstraction over file system operations in
MiniMake, allowing tests to use an in-memory file system with precise
control over modification times.

https://apenwarr.ca/log/20181113
  • Loading branch information
kateinoigakukun committed Oct 19, 2025
commit 4b7292430c6c3f7c25972355529edd588cfc28f9
51 changes: 39 additions & 12 deletions Plugins/PackageToJS/Sources/MiniMake.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,18 @@ struct MiniMake {
private var tasks: [TaskKey: Task]
/// Whether to explain why tasks are built
private var shouldExplain: Bool
/// File system operations
private var fileSystem: MiniMakeFileSystem
/// Prints progress of the build
private var printProgress: ProgressPrinter.PrintProgress

init(
explain: Bool = false,
fileSystem: MiniMakeFileSystem = DefaultMiniMakeFileSystem(),
printProgress: @escaping ProgressPrinter.PrintProgress
) {
self.tasks = [:]
self.fileSystem = fileSystem
self.shouldExplain = explain
self.printProgress = printProgress
}
Expand Down Expand Up @@ -222,7 +226,7 @@ struct MiniMake {
/// Cleans all outputs of all tasks
func cleanEverything(scope: VariableScope) {
for task in self.tasks.values {
try? FileManager.default.removeItem(at: scope.resolve(path: task.output))
try? fileSystem.removeItem(at: scope.resolve(path: task.output))
}
}

Expand All @@ -234,26 +238,20 @@ struct MiniMake {
return true
}
let outputURL = scope.resolve(path: task.output)
if !FileManager.default.fileExists(atPath: outputURL.path) {
if !fileSystem.fileExists(at: outputURL) {
explain("Task \(task.output) should be built because it doesn't exist")
return true
}
let outputMtime = try? outputURL.resourceValues(forKeys: [.contentModificationDateKey])
.contentModificationDate
let outputMtime = try? fileSystem.modificationDate(of: outputURL)
return task.inputs.contains { input in
let inputURL = scope.resolve(path: input)
// Ignore directory modification times
var isDirectory: ObjCBool = false
let fileExists = FileManager.default.fileExists(
atPath: inputURL.path,
isDirectory: &isDirectory
)
if fileExists && isDirectory.boolValue {
let (fileExists, isDirectory) = fileSystem.fileExists(at: inputURL)
if fileExists && isDirectory {
return false
}

let inputMtime = try? inputURL.resourceValues(forKeys: [.contentModificationDateKey]
).contentModificationDate
let inputMtime = try? fileSystem.modificationDate(of: inputURL)
let shouldBuild =
outputMtime == nil || inputMtime == nil || outputMtime! < inputMtime!
if shouldBuild {
Expand Down Expand Up @@ -337,3 +335,32 @@ struct BuildPath: Encodable, Hashable, CustomStringConvertible {
try container.encode(self.description)
}
}

/// Abstraction over file system operations
protocol MiniMakeFileSystem {
func removeItem(at url: URL) throws
func fileExists(at url: URL) -> Bool
func fileExists(at url: URL) -> (exists: Bool, isDirectory: Bool)
func modificationDate(of url: URL) throws -> Date?
}

/// Default implementation of MiniMakeFileSystem using FileManager
struct DefaultMiniMakeFileSystem: MiniMakeFileSystem {
func removeItem(at url: URL) throws {
try FileManager.default.removeItem(at: url)
}

func fileExists(at url: URL) -> Bool {
FileManager.default.fileExists(atPath: url.path)
}

func fileExists(at url: URL) -> (exists: Bool, isDirectory: Bool) {
var isDirectory: ObjCBool = false
let exists = FileManager.default.fileExists(atPath: url.path, isDirectory: &isDirectory)
return (exists, isDirectory.boolValue)
}

func modificationDate(of url: URL) throws -> Date? {
try url.resourceValues(forKeys: [.contentModificationDateKey]).contentModificationDate
}
}
81 changes: 73 additions & 8 deletions Plugins/PackageToJS/Tests/MiniMakeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,67 @@ import Testing
@testable import PackageToJS

@Suite struct MiniMakeTests {
final class InMemoryFileSystem: MiniMakeFileSystem {
struct FileEntry {
var content: Data
var modificationDate: Date
var isDirectory: Bool
}
private var storage: [URL: FileEntry] = [:]

struct MonotonicDateGenerator {
private var currentDate: Date

init(startingFrom date: Date = Date()) {
self.currentDate = date
}

mutating func next() -> Date {
currentDate = currentDate.addingTimeInterval(1)
return currentDate
}
}
var dateGenerator = MonotonicDateGenerator()

// MARK: - MiniMakeFileSystem conformance

func removeItem(at url: URL) throws {
storage.removeValue(forKey: url)
}

func fileExists(at url: URL) -> Bool {
return storage[url] != nil
}

func fileExists(at url: URL) -> (exists: Bool, isDirectory: Bool) {
if let entry = storage[url] {
return (true, entry.isDirectory)
} else {
return (false, false)
}
}

func modificationDate(of url: URL) throws -> Date? {
return storage[url]?.modificationDate
}

func writeFile(at url: URL, content: Data) {
storage[url] = FileEntry(content: content, modificationDate: dateGenerator.next(), isDirectory: false)
}

// MARK: - Helpers for tests

func touch(_ url: URL) {
let date = dateGenerator.next()
if var entry = storage[url] {
entry.modificationDate = date
storage[url] = entry
} else {
storage[url] = FileEntry(content: Data(), modificationDate: date, isDirectory: false)
}
}
}

// Test basic task management functionality
@Test func basicTaskManagement() throws {
try withTemporaryDirectory { tempDir, _ in
Expand Down Expand Up @@ -114,7 +175,11 @@ import Testing
// Test that rebuilds are controlled by timestamps
@Test func timestampBasedRebuild() throws {
try withTemporaryDirectory { tempDir, _ in
var make = MiniMake(printProgress: { _, _ in })
let fs = InMemoryFileSystem()
var make = MiniMake(
fileSystem: fs,
printProgress: { _, _ in }
)
let prefix = BuildPath(prefix: "PREFIX")
let scope = MiniMake.VariableScope(variables: [
"PREFIX": tempDir.path
Expand All @@ -123,25 +188,25 @@ import Testing
let output = prefix.appending(path: "output.txt")
var buildCount = 0

try "Initial".write(toFile: scope.resolve(path: input).path, atomically: true, encoding: .utf8)
// Create initial input file
fs.touch(scope.resolve(path: input))

let task = make.addTask(inputFiles: [input], output: output) { task, scope in
buildCount += 1
let content = try String(contentsOfFile: scope.resolve(path: task.inputs[0]).path, encoding: .utf8)
try content.write(toFile: scope.resolve(path: task.output).path, atomically: true, encoding: .utf8)
fs.touch(scope.resolve(path: task.output))
}

// First build
try make.build(output: task, scope: scope)
#expect(throws: Never.self) { try make.build(output: task, scope: scope) }
#expect(buildCount == 1, "First build should occur")

// Second build without changes
try make.build(output: task, scope: scope)
#expect(throws: Never.self) { try make.build(output: task, scope: scope) }
#expect(buildCount == 1, "No rebuild should occur if input is not modified")

// Modify input and rebuild
try "Modified".write(toFile: scope.resolve(path: input).path, atomically: true, encoding: .utf8)
try make.build(output: task, scope: scope)
fs.touch(scope.resolve(path: input))
#expect(throws: Never.self) { try make.build(output: task, scope: scope) }
#expect(buildCount == 2, "Should rebuild when input is modified")
}
}
Expand Down