Skip to content

Commit 72d64bd

Browse files
Adam Fiskclaude
andcommitted
logging: test that log lines accumulate instead of overwriting
148cdf7 replaced seekToEndOfFile() + write(data) with a POSIX append and dropped the seek. FileHandle(forWritingTo:) opens at offset zero and write(2) writes at the current offset, so every line overwrote from the start and the file was left holding only the most recent one. 0b09c54 fixed it with an lseek to SEEK_END under the append lock. Nothing would have caught that. It compiles cleanly, and the round-trip check used to justify the original change opened one descriptor, wrote once and read back -- on an empty file offset zero already is the end, so a single write passes with or without the seek. The bug only appears across repeated open/write/close cycles, which is the actual usage pattern. So test that pattern: write five lines through the logger and assert all five survive. Verified it fails on the regression -- removing the lseek turns it from passing in 0.022s into failing after the full poll deadline. Testing it needs a writable directory, so the log location becomes an init parameter with a convenience init() preserving FilePath.logsDirectory for production. Applied to both platforms to keep the two files in step, though only macOS has a test target to run it. Verified locally on both: make macos-unit-tests reports TEST SUCCEEDED, and make ios-compile-check builds Runner.app. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0b09c54 commit 72d64bd

3 files changed

Lines changed: 62 additions & 4 deletions

File tree

‎ios/Shared/Logger.swift‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,17 @@ class LanternLogger {
3737
/// Bytes appended since the last size check, so we don't stat on every line.
3838
private var sinceCheck = 0
3939

40-
init() {
40+
convenience init() {
41+
self.init(logsDirectory: FilePath.logsDirectory)
42+
}
43+
44+
/// Designated initializer. The directory is a parameter so tests can drive
45+
/// the real write path against a temporary location -- appends only reveal
46+
/// themselves across repeated open/write/close cycles, which is exactly what
47+
/// a unit test can reproduce and a compile check cannot.
48+
init(logsDirectory: URL) {
4149
// Ensure Logs directory exists
42-
let logsDir = FilePath.logsDirectory
50+
let logsDir = logsDirectory
4351
if !FileManager.default.fileExists(atPath: logsDir.path) {
4452
try? FileManager.default.createDirectory(at: logsDir, withIntermediateDirectories: true)
4553
}

‎macos/RunnerTests/RunnerTests.swift‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,48 @@ import XCTest
77

88
final class RunnerTests: XCTestCase {
99

10+
/// Every log line must survive. FileHandle(forWritingTo:) opens at offset 0,
11+
/// so a write that does not seek to the end first overwrites from the start
12+
/// and the file ends up holding only the most recent line. That regression
13+
/// shipped briefly and compiled cleanly -- it is invisible to a build and to
14+
/// any test that writes exactly once, because on an empty file offset 0 is
15+
/// already the end. It only shows up across repeated open/write/close cycles,
16+
/// which is the real usage pattern, so drive several writes and assert that
17+
/// all of them are still there.
18+
func testLogLinesAccumulateAcrossWrites() throws {
19+
let directory = URL(fileURLWithPath: NSTemporaryDirectory())
20+
.appendingPathComponent("lantern-logger-\(UUID().uuidString)")
21+
try FileManager.default.createDirectory(
22+
at: directory, withIntermediateDirectories: true)
23+
defer { try? FileManager.default.removeItem(at: directory) }
24+
25+
let logger = LanternLogger(logsDirectory: directory)
26+
let lines = (1...5).map { "accumulate-marker-\($0)" }
27+
for line in lines {
28+
logger.log(line)
29+
}
30+
31+
// Writes are dispatched asynchronously onto the logger's serial queue, so
32+
// poll rather than assuming they have landed.
33+
let logFile = directory.appendingPathComponent("lantern_macos.log")
34+
let deadline = Date().addingTimeInterval(10)
35+
var contents = ""
36+
while Date() < deadline {
37+
contents = (try? String(contentsOf: logFile, encoding: .utf8)) ?? ""
38+
if lines.allSatisfy({ contents.contains($0) }) {
39+
break
40+
}
41+
usleep(20_000)
42+
}
43+
44+
for line in lines {
45+
XCTAssertTrue(
46+
contents.contains(line),
47+
"\(line) is missing, so appends are overwriting instead of appending. "
48+
+ "File held: \(contents.debugDescription)")
49+
}
50+
}
51+
1052
private func assertVPNManagerError(
1153
_ expected: VPNManagerError,
1254
_ operation: () throws -> Void

‎macos/Shared/Logger.swift‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,16 @@ class LanternLogger {
4141
/// Bytes appended since the last size check, so we don't stat on every line.
4242
private var sinceCheck = 0
4343

44-
init() {
45-
let logsDir = FilePath.logsDirectory
44+
convenience init() {
45+
self.init(logsDirectory: FilePath.logsDirectory)
46+
}
47+
48+
/// Designated initializer. The directory is a parameter so tests can drive
49+
/// the real write path against a temporary location -- appends only reveal
50+
/// themselves across repeated open/write/close cycles, which is exactly what
51+
/// a unit test can reproduce and a compile check cannot.
52+
init(logsDirectory: URL) {
53+
let logsDir = logsDirectory
4654
if !FileManager.default.fileExists(atPath: logsDir.path) {
4755
try? FileManager.default.createDirectory(at: logsDir, withIntermediateDirectories: true)
4856
}

0 commit comments

Comments
 (0)