Skip to content

Commit d44aa68

Browse files
committed
refactor tests around actor data handlers
1 parent 617593f commit d44aa68

16 files changed

+659
-688
lines changed

AGENT.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,13 @@
1818
- Command-line full test runs should use serial execution:
1919
- `swift test --no-parallel`
2020
- or `./test.sh`
21+
- `./test.sh` is the preferred full-suite command because it enables Core Data concurrency assertions via `com.apple.CoreData.ConcurrencyDebug=1`.
2122
- In Xcode, disable parallel testing for this package before running the full suite.
2223
- When debugging failures, prefer running a single test file or a filtered suite first.
2324
- This repository's tests are currently treated as serial-only at the full-suite level.
25+
- For new Core Data tests, do not write directly through `viewContext` or a raw background context from the test body.
26+
- Prefer `Tests/PersistentHistoryTrackingKitTests/TestAppDataHandler.swift` and actor-isolated helper methods for creating, updating, deleting, and reading test data.
27+
- If a test needs direct inspection of a handler-owned context, use the handler's `withContext` API rather than `context.perform` from the test body.
2428

2529
## Known Test Constraints
2630

Sources/PersistentHistoryTrackingKit/PersistentHistoryTrackingKit.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ public final class PersistentHistoryTrackingKit: @unchecked Sendable {
126126

127127
transactionProcessor = TransactionProcessorActor(
128128
container: container,
129+
contexts: self.contexts,
129130
hookRegistry: hookRegistry,
130131
cleanStrategy: cleanStrategy,
131132
timestampManager: timestampManager)
@@ -345,7 +346,6 @@ public final class PersistentHistoryTrackingKit: @unchecked Sendable {
345346
.processNewTransactionsWithTimestampManagement(
346347
from: authorsToProcess,
347348
after: lastTimestamp,
348-
mergeInto: contexts,
349349
currentAuthor: currentAuthor,
350350
batchAuthors: batchAuthors)
351351

Sources/PersistentHistoryTrackingKit/TransactionProcessorActor+Testing.swift

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,13 @@ import Foundation
7676
/// - Parameters:
7777
/// - authors: Authors whose transactions should be processed.
7878
/// - lastTimestamp: Last processed timestamp.
79-
/// - contexts: Target contexts.
8079
/// - currentAuthor: Current author.
8180
/// - cleanBeforeTimestamp: Cleanup cutoff timestamp.
8281
/// - expectedEntityName: Expected entity name for validation.
8382
/// - Returns: (transaction count, first transaction author, first change entity name)
8483
func testProcessNewTransactions(
8584
from authors: [String],
8685
after lastTimestamp: Date?,
87-
mergeInto contexts: [NSManagedObjectContext],
8886
currentAuthor: String?,
8987
cleanBeforeTimestamp: Date?,
9088
expectedEntityName: String?
@@ -113,7 +111,6 @@ import Foundation
113111
let count = try await processNewTransactions(
114112
from: authors,
115113
after: lastTimestamp,
116-
mergeInto: contexts,
117114
currentAuthor: currentAuthor,
118115
cleanBeforeTimestamp: cleanBeforeTimestamp)
119116

@@ -165,14 +162,12 @@ import Foundation
165162
/// Verify that hooks are triggered as expected.
166163
/// - Parameters:
167164
/// - authors: Authors to process.
168-
/// - contexts: Contexts to merge into.
169165
/// - expectedEntityName: Expected entity name for the hook.
170166
/// - expectedOperation: Expected operation type.
171167
/// - Returns: (transaction count, first change entity name, first change operation)
172168
func testHookTrigger(
173169
from authors: [String],
174170
after date: Date?,
175-
mergeInto contexts: [NSManagedObjectContext],
176171
currentAuthor: String?,
177172
expectedEntityName: String,
178173
expectedOperation: HookOperation
@@ -205,7 +200,6 @@ import Foundation
205200
let count = try await processNewTransactions(
206201
from: authors,
207202
after: date,
208-
mergeInto: contexts,
209203
currentAuthor: currentAuthor,
210204
cleanBeforeTimestamp: nil)
211205

Sources/PersistentHistoryTrackingKit/TransactionProcessorActor.swift

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
// Copyright © 2025 Yang Xu. All rights reserved.
77
//
88

9-
import CoreData
109
import CoreDataEvolution
1110
import Foundation
1211

@@ -22,6 +21,9 @@ actor TransactionProcessorActor {
2221
/// Timestamp manager.
2322
private let timestampManager: TransactionTimestampManager
2423

24+
/// Contexts that receive merged transactions.
25+
private let mergeContexts: [NSManagedObjectContext]
26+
2527
// MARK: - Merge Hooks (managed inside the same actor to avoid passing non-Sendable types)
2628

2729
private struct MergeHookItem {
@@ -33,12 +35,14 @@ actor TransactionProcessorActor {
3335

3436
init(
3537
container: NSPersistentContainer,
38+
contexts: [NSManagedObjectContext]? = nil,
3639
hookRegistry: HookRegistryActor,
3740
cleanStrategy: TransactionCleanStrategy,
3841
timestampManager: consuming TransactionTimestampManager
3942
) {
4043
self.hookRegistry = hookRegistry
4144
self.timestampManager = timestampManager
45+
mergeContexts = contexts ?? [container.viewContext]
4246

4347
// Initialize the cleanup strategy configuration.
4448
switch cleanStrategy {
@@ -101,15 +105,13 @@ actor TransactionProcessorActor {
101105
/// - Parameters:
102106
/// - authors: Authors whose transactions need to be merged.
103107
/// - lastTimestamp: The last processed timestamp.
104-
/// - contexts: Target contexts for merging.
105108
/// - currentAuthor: Current author (used to exclude self-authored transactions).
106109
/// - cleanBeforeTimestamp: Optional timestamp; history before this will be cleaned.
107110
/// - Returns: Number of transactions processed.
108111
@discardableResult
109112
func processNewTransactions(
110113
from authors: [String],
111114
after lastTimestamp: Date?,
112-
mergeInto contexts: [NSManagedObjectContext],
113115
currentAuthor: String? = nil,
114116
cleanBeforeTimestamp: Date? = nil
115117
) async throws -> Int {
@@ -125,7 +127,7 @@ actor TransactionProcessorActor {
125127

126128
// 3. Trigger Merge Hooks (pipeline, may mutate data).
127129
// Runs within the same actor to avoid cross-actor non-Sendable transfers.
128-
try await triggerMergeHooks(transactions: transactions, contexts: contexts)
130+
try await triggerMergeHooks(transactions: transactions)
129131

130132
// 4. Run cleanup when a timestamp is provided.
131133
if let cleanTimestamp = cleanBeforeTimestamp {
@@ -139,15 +141,13 @@ actor TransactionProcessorActor {
139141
/// - Parameters:
140142
/// - authors: Authors whose transactions need to be merged.
141143
/// - lastTimestamp: The last processed timestamp.
142-
/// - contexts: Target contexts for merging.
143144
/// - currentAuthor: Current author (excluded from fetch and used to update timestamp).
144145
/// - batchAuthors: Authors participating in batch work (excluded from cleanup calculation).
145146
/// - Returns: Number of transactions processed.
146147
@discardableResult
147148
func processNewTransactionsWithTimestampManagement(
148149
from authors: [String],
149150
after lastTimestamp: Date?,
150-
mergeInto contexts: [NSManagedObjectContext],
151151
currentAuthor: String,
152152
batchAuthors: [String] = []
153153
) async throws -> Int {
@@ -162,7 +162,7 @@ actor TransactionProcessorActor {
162162
await triggerObserverHooks(for: transactions)
163163

164164
// 3. Trigger Merge Hooks (pipeline that may mutate data).
165-
try await triggerMergeHooks(transactions: transactions, contexts: contexts)
165+
try await triggerMergeHooks(transactions: transactions)
166166

167167
// 4. Update the timestamp for the current author.
168168
if let newTimestamp = transactions.last?.timestamp {
@@ -358,13 +358,11 @@ actor TransactionProcessorActor {
358358
/// Run the Merge Hook pipeline.
359359
/// - Parameters:
360360
/// - transactions: Transactions flowing through the pipeline.
361-
/// - contexts: Target contexts.
362361
/// - Note: All operations execute inside the same actor, so there are no concurrency concerns.
363362
private func triggerMergeHooks(
364-
transactions: [NSPersistentHistoryTransaction],
365-
contexts: [NSManagedObjectContext]
363+
transactions: [NSPersistentHistoryTransaction]
366364
) async throws {
367-
let input = MergeHookInput(transactions: transactions, contexts: contexts)
365+
let input = MergeHookInput(transactions: transactions, contexts: mergeContexts)
368366

369367
// Execute each merge hook sequentially.
370368
for item in mergeHooks {
@@ -375,7 +373,7 @@ actor TransactionProcessorActor {
375373
}
376374

377375
// All hooks returned .goOn (or none were registered), so perform the default merge.
378-
try await mergeTransactions(transactions, into: contexts)
376+
try await mergeTransactions(transactions, into: mergeContexts)
379377
}
380378

381379
// MARK: - Clean

Tests/PersistentHistoryTrackingKitTests/CleanStrategyTests.swift

Lines changed: 18 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -17,25 +17,19 @@ struct CleanStrategyTests {
1717
let container = TestModelBuilder.createContainer(
1818
author: "App1",
1919
testName: "noneStrategyDisablesAutomaticCleanup")
20-
let context1 = container.viewContext
21-
context1.transactionAuthor = "App1"
20+
let writer = TestAppDataHandler(container: container, viewName: "Writer")
2221

23-
TestModelBuilder.createPerson(name: "Alice", age: 30, in: context1)
24-
try context1.save()
22+
try await writer.createPerson(name: "Alice", age: 30, author: "App1")
2523

2624
try await Task.sleep(nanoseconds: 100_000_000)
2725

28-
TestModelBuilder.createPerson(name: "Bob", age: 31, in: context1)
29-
try context1.save()
26+
try await writer.createPerson(name: "Bob", age: 31, author: "App1")
3027

3128
let processor = makeProcessor(container: container, cleanStrategy: .none)
32-
let mergeContext = container.newBackgroundContext()
33-
mergeContext.transactionAuthor = "App2"
3429

3530
_ = try await processor.processNewTransactionsWithTimestampManagement(
3631
from: ["App1", "App2"],
3732
after: nil,
38-
mergeInto: [mergeContext],
3933
currentAuthor: "App2")
4034

4135
let remainingCount = try await processor.testTransactionCount(
@@ -51,30 +45,24 @@ struct CleanStrategyTests {
5145
author: "App1",
5246
testName: "durationStrategyThrottlesCleanup")
5347
let userDefaults = TestModelBuilder.createTestUserDefaults()
54-
let context1 = container.viewContext
55-
context1.transactionAuthor = "App1"
48+
let writer = TestAppDataHandler(container: container, viewName: "Writer")
5649

57-
TestModelBuilder.createPerson(name: "Alice", age: 30, in: context1)
58-
try context1.save()
50+
try await writer.createPerson(name: "Alice", age: 30, author: "App1")
5951

6052
try await Task.sleep(nanoseconds: 100_000_000)
6153

62-
TestModelBuilder.createPerson(name: "Bob", age: 31, in: context1)
63-
try context1.save()
54+
try await writer.createPerson(name: "Bob", age: 31, author: "App1")
6455

6556
let processor = makeProcessor(
6657
container: container,
6758
userDefaults: userDefaults,
6859
cleanStrategy: .byDuration(seconds: 60 * 60))
69-
let mergeContext = container.newBackgroundContext()
70-
mergeContext.transactionAuthor = "App2"
7160

7261
userDefaults.set(Date(), forKey: "PersistentHistoryTrackingKit.lastToken.App1")
7362

7463
_ = try await processor.processNewTransactionsWithTimestampManagement(
7564
from: ["App1", "App2"],
7665
after: nil,
77-
mergeInto: [mergeContext],
7866
currentAuthor: "App2")
7967

8068
let countAfterFirstProcess = try await processor.testTransactionCount(
@@ -87,13 +75,11 @@ struct CleanStrategyTests {
8775

8876
try await Task.sleep(nanoseconds: 100_000_000)
8977

90-
TestModelBuilder.createPerson(name: "Charlie", age: 32, in: context1)
91-
try context1.save()
78+
try await writer.createPerson(name: "Charlie", age: 32, author: "App1")
9279

9380
_ = try await processor.processNewTransactionsWithTimestampManagement(
9481
from: ["App1", "App2"],
9582
after: lastTimestamp,
96-
mergeInto: [mergeContext],
9783
currentAuthor: "App2")
9884

9985
let countAfterSecondProcess = try await processor.testTransactionCount(
@@ -108,30 +94,24 @@ struct CleanStrategyTests {
10894
author: "App1",
10995
testName: "notificationStrategyCleansOnConfiguredCount")
11096
let userDefaults = TestModelBuilder.createTestUserDefaults()
111-
let context1 = container.viewContext
112-
context1.transactionAuthor = "App1"
97+
let writer = TestAppDataHandler(container: container, viewName: "Writer")
11398

114-
TestModelBuilder.createPerson(name: "Alice", age: 30, in: context1)
115-
try context1.save()
99+
try await writer.createPerson(name: "Alice", age: 30, author: "App1")
116100

117101
try await Task.sleep(nanoseconds: 100_000_000)
118102

119-
TestModelBuilder.createPerson(name: "Bob", age: 31, in: context1)
120-
try context1.save()
103+
try await writer.createPerson(name: "Bob", age: 31, author: "App1")
121104

122105
let processor = makeProcessor(
123106
container: container,
124107
userDefaults: userDefaults,
125108
cleanStrategy: .byNotification(times: 2))
126-
let mergeContext = container.newBackgroundContext()
127-
mergeContext.transactionAuthor = "App2"
128109

129110
userDefaults.set(Date(), forKey: "PersistentHistoryTrackingKit.lastToken.App1")
130111

131112
_ = try await processor.processNewTransactionsWithTimestampManagement(
132113
from: ["App1", "App2"],
133114
after: nil,
134-
mergeInto: [mergeContext],
135115
currentAuthor: "App2")
136116

137117
let countAfterFirstProcess = try await processor.testTransactionCount(
@@ -144,13 +124,11 @@ struct CleanStrategyTests {
144124

145125
try await Task.sleep(nanoseconds: 100_000_000)
146126

147-
TestModelBuilder.createPerson(name: "Charlie", age: 32, in: context1)
148-
try context1.save()
127+
try await writer.createPerson(name: "Charlie", age: 32, author: "App1")
149128

150129
_ = try await processor.processNewTransactionsWithTimestampManagement(
151130
from: ["App1", "App2"],
152131
after: lastTimestamp,
153-
mergeInto: [mergeContext],
154132
currentAuthor: "App2")
155133

156134
let countAfterSecondProcess = try await processor.testTransactionCount(
@@ -164,27 +142,21 @@ struct CleanStrategyTests {
164142
let container = TestModelBuilder.createContainer(
165143
author: "App1",
166144
testName: "automaticCleanupWaitsForMissingAuthorTimestamps")
167-
let context1 = container.viewContext
168-
context1.transactionAuthor = "App1"
145+
let writer = TestAppDataHandler(container: container, viewName: "Writer")
169146

170-
TestModelBuilder.createPerson(name: "Alice", age: 30, in: context1)
171-
try context1.save()
147+
try await writer.createPerson(name: "Alice", age: 30, author: "App1")
172148

173149
try await Task.sleep(nanoseconds: 100_000_000)
174150

175-
TestModelBuilder.createPerson(name: "Bob", age: 31, in: context1)
176-
try context1.save()
151+
try await writer.createPerson(name: "Bob", age: 31, author: "App1")
177152

178153
let processor = makeProcessor(
179154
container: container,
180155
cleanStrategy: .byDuration(seconds: 60 * 60))
181-
let mergeContext = container.newBackgroundContext()
182-
mergeContext.transactionAuthor = "App2"
183156

184157
_ = try await processor.processNewTransactionsWithTimestampManagement(
185158
from: ["App1", "App2", "App3"],
186159
after: nil,
187-
mergeInto: [mergeContext],
188160
currentAuthor: "App2")
189161

190162
let remainingCount = try await processor.testTransactionCount(
@@ -200,30 +172,24 @@ struct CleanStrategyTests {
200172
author: "App1",
201173
testName: "batchAuthorsDoNotBlockCleanupReadiness")
202174
let userDefaults = TestModelBuilder.createTestUserDefaults()
203-
let context1 = container.viewContext
204-
context1.transactionAuthor = "App1"
175+
let writer = TestAppDataHandler(container: container, viewName: "Writer")
205176

206-
TestModelBuilder.createPerson(name: "Alice", age: 30, in: context1)
207-
try context1.save()
177+
try await writer.createPerson(name: "Alice", age: 30, author: "App1")
208178

209179
try await Task.sleep(nanoseconds: 100_000_000)
210180

211-
TestModelBuilder.createPerson(name: "Bob", age: 31, in: context1)
212-
try context1.save()
181+
try await writer.createPerson(name: "Bob", age: 31, author: "App1")
213182

214183
let processor = makeProcessor(
215184
container: container,
216185
userDefaults: userDefaults,
217186
cleanStrategy: .byDuration(seconds: 60 * 60))
218-
let mergeContext = container.newBackgroundContext()
219-
mergeContext.transactionAuthor = "App2"
220187

221188
userDefaults.set(Date(), forKey: "PersistentHistoryTrackingKit.lastToken.App1")
222189

223190
_ = try await processor.processNewTransactionsWithTimestampManagement(
224191
from: ["App1", "App2", "BatchProcessor"],
225192
after: nil,
226-
mergeInto: [mergeContext],
227193
currentAuthor: "App2",
228194
batchAuthors: ["BatchProcessor"])
229195

@@ -245,6 +211,7 @@ struct CleanStrategyTests {
245211
maximumDuration: 604_800)
246212
return TransactionProcessorActor(
247213
container: container,
214+
contexts: [container.newBackgroundContext()],
248215
hookRegistry: hookRegistry,
249216
cleanStrategy: cleanStrategy,
250217
timestampManager: timestampManager)

0 commit comments

Comments
 (0)