Commit 982ec60b32

982ec60b32f7d9ecde0dfbad61d0325cd8a18c53

parent: 4d6ea462de

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-07 02:08 UTC

Replace fixed-sleep waits with polling in reload tests

Eleven tests changed a filter, slept a fixed interval, then asserted
on stub.seen.last. Under load the reload lands after the sleep expires,
and two of the cursor assertions passed vacuously even when the reload
never happened at all (the initial request also carries no cursor).

Add `until` to StubProtocol.swift: poll a condition instead of sleeping
a fixed interval, and rewrite each of the eleven to wait on the actual
expected request count or state, asserting on the request identified by
its argv rather than on whatever is last.

Layout: unified · split

gitbayTests/ListFilterTests.swift +39 −8
@@ -184,7 +184,7 @@ struct IssueListFilterWiringTests {
184 184
185 stub.enqueue(.init(status: 200, json: emptyPage, match: "argv=issue")) 185 stub.enqueue(.init(status: 200, json: emptyPage, match: "argv=issue"))
186 model.filter.label = "bug" 186 model.filter.label = "bug"
187 try await Task.sleep(for: .milliseconds(150)) 187 await until { stub.seen.filter { $0.url.absoluteString.contains("argv=issue") }.count == 2 }
188 188
189 let reads = stub.seen.filter { $0.url.absoluteString.contains("argv=issue") } 189 let reads = stub.seen.filter { $0.url.absoluteString.contains("argv=issue") }
190 let last = try #require(reads.last) 190 let last = try #require(reads.last)
@@ -275,11 +275,18 @@ struct IssueListFilterWiringTests {
275 275
276 stub.enqueue(.init(status: 200, json: emptyPage, match: "argv=issue")) 276 stub.enqueue(.init(status: 200, json: emptyPage, match: "argv=issue"))
277 model.filter.label = "bug" 277 model.filter.label = "bug"
278 try await Task.sleep(for: .milliseconds(150)) 278 // Poll for the read that actually carries the new filter, not
279 // whatever happens to be last: a reload that never landed would
280 // otherwise leave `last` pointing at the initial request, which
281 // also carries no cursor and would pass just as wrongly.
282 await until {
283 stub.seen.contains { $0.url.absoluteString.contains("argv=issue") && argvFrom($0.url).contains("bug") }
284 }
279 285
280 let reads = stub.seen.filter { $0.url.absoluteString.contains("argv=issue") } 286 let reload = try #require(stub.seen.first {
281 let last = try #require(reads.last) 287 $0.url.absoluteString.contains("argv=issue") && argvFrom($0.url).contains("bug")
282 #expect(!argvFrom(last.url).contains("--cursor")) 288 })
289 #expect(!argvFrom(reload.url).contains("--cursor"))
283 } 290 }
284 291
285 /// Two rapid picker changes cancel the first reload's task. The 292 /// Two rapid picker changes cancel the first reload's task. The
@@ -296,7 +303,31 @@ struct IssueListFilterWiringTests {
296 stub.enqueue(.init(status: 200, json: emptyPage, match: "argv=issue")) 303 stub.enqueue(.init(status: 200, json: emptyPage, match: "argv=issue"))
297 model.filter.label = "bug" 304 model.filter.label = "bug"
298 model.filter.label = "feature" 305 model.filter.label = "feature"
299 try await Task.sleep(for: .milliseconds(200)) 306 // Wait for the winning reload's read to land rather than a fixed
307 // sleep: checking `.failed` against a still-untouched state (the
308 // spawned task hasn't run yet) would pass no matter what the real
309 // behaviour is. Whether the cancelled first task's own read also
310 // reaches the stub before it notices cancellation is a race in
311 // its own right, so this only requires the winning one.
312 await until {
313 stub.seen.filter { $0.url.absoluteString.contains("argv=issue") }.count >= 2
314 }
315 // The cancelled task's own completion can still land after the
316 // winning reload's; a poll alone stops the instant the count is
317 // satisfied and would miss a bogus overwrite arriving right
318 // after. A short settle catches it.
319 try await Task.sleep(for: .milliseconds(50))
320
321 // The state the poll waited for is "a reload's read landed", not
322 // "the screen settled" — a filter change that reloaded nothing at
323 // all would leave the pre-existing, unchanged, non-failed state
324 // sitting there and pass just as wrongly as a `.loading` screen
325 // checked too early would.
326 let reads = stub.seen.filter { $0.url.absoluteString.contains("argv=issue") }.count
327 guard reads >= 2 else {
328 Testing.Issue.record("rapid filter changes never reloaded")
329 return
330 }
300 331
301 if case .failed(let message) = model.state { 332 if case .failed(let message) = model.state {
302 Testing.Issue.record("rapid filter changes left the screen failed: \(message)") 333 Testing.Issue.record("rapid filter changes left the screen failed: \(message)")
@@ -329,7 +360,7 @@ struct MRListFilterWiringTests {
329 360
330 stub.enqueue(.init(status: 200, json: emptyPage)) 361 stub.enqueue(.init(status: 200, json: emptyPage))
331 model.filter.search = "retarget" 362 model.filter.search = "retarget"
332 try await Task.sleep(for: .milliseconds(500)) 363 await until { stub.seen.count == 2 }
333 364
334 let last = try #require(stub.seen.last) 365 let last = try #require(stub.seen.last)
335 let argv = argvFrom(last.url) 366 let argv = argvFrom(last.url)
@@ -366,7 +397,7 @@ struct MRListFilterWiringTests {
366 397
367 stub.enqueue(.init(status: 200, json: emptyPage)) 398 stub.enqueue(.init(status: 200, json: emptyPage))
368 model.filter.state = .sourceGone 399 model.filter.state = .sourceGone
369 try await Task.sleep(for: .milliseconds(150)) 400 await until { stub.seen.count == 2 }
370 401
371 #expect(argvFrom(try #require(stub.seen.last).url).contains("source_gone")) 402 #expect(argvFrom(try #require(stub.seen.last).url).contains("source_gone"))
372 } 403 }
gitbayTests/MRViewModelTests.swift +11 −2
@@ -172,8 +172,17 @@ struct MRListViewModelTests {
172 await model.load() 172 await model.load()
173 173
174 model.filter.state = .merged 174 model.filter.state = .merged
175 // The reload happens in a spawned task; give it a beat. 175 // The reload happens in a spawned task; wait for it to settle
176 try await Task.sleep(for: .milliseconds(300)) 176 // rather than for a fixed interval to pass. Checking `model.state`
177 // alone is not enough: right after the assignment, before the
178 // spawned task has even run, the state is still the *previous*
179 // load's non-loading result, which would satisfy a bare
180 // "not loading" check before the reload ever started.
181 await until {
182 guard stub.seen.count == 2 else { return false }
183 if case .loading = model.state { return false }
184 return true
185 }
177 186
178 #expect(stub.seen.count == 2) 187 #expect(stub.seen.count == 2)
179 #expect(stub.seen[1].url.query()?.contains("argv=merged") == true) 188 #expect(stub.seen[1].url.query()?.contains("argv=merged") == true)
gitbayTests/NotificationTests.swift +9 −5
@@ -127,7 +127,7 @@ struct NotificationsViewModelTests {
127 127
128 stub.enqueue(.init(status: 200, json: page)) 128 stub.enqueue(.init(status: 200, json: page))
129 model.showAll = true 129 model.showAll = true
130 try await Task.sleep(for: .milliseconds(150)) 130 await until { stub.seen.count == 2 }
131 131
132 #expect(argvFrom(try #require(stub.seen.last).url).contains("--all")) 132 #expect(argvFrom(try #require(stub.seen.last).url).contains("--all"))
133 } 133 }
@@ -143,10 +143,14 @@ struct NotificationsViewModelTests {
143 143
144 stub.enqueue(.init(status: 200, json: page)) 144 stub.enqueue(.init(status: 200, json: page))
145 model.showAll = true 145 model.showAll = true
146 try await Task.sleep(for: .milliseconds(150)) 146 // Poll for the read that actually carries the new filter, not
147 147 // whatever happens to be last: a reload that never landed would
148 let argv = argvFrom(try #require(stub.seen.last).url) 148 // otherwise leave `last` pointing at the initial request, which
149 #expect(argv.contains("--cursor") == false) 149 // also carries no cursor and would pass just as wrongly.
150 await until { stub.seen.contains { argvFrom($0.url).contains("--all") } }
151
152 let reload = try #require(stub.seen.first { argvFrom($0.url).contains("--all") })
153 #expect(!argvFrom(reload.url).contains("--cursor"))
150 } 154 }
151 155
152 @Test func markingOneReadSendsItsId() async throws { 156 @Test func markingOneReadSendsItsId() async throws {
gitbayTests/RefsMilestoneTests.swift +11 −1
@@ -107,7 +107,17 @@ struct MilestoneListViewModelTests {
107 await model.load() 107 await model.load()
108 108
109 model.filter = .closed 109 model.filter = .closed
110 try await Task.sleep(for: .milliseconds(300)) 110 // The reload happens in a spawned task; wait for it to settle
111 // rather than for a fixed interval to pass. Checking `model.state`
112 // alone is not enough: right after the assignment, before the
113 // spawned task has even run, the state is still the *previous*
114 // load's non-loading result, which would satisfy a bare
115 // "not loading" check before the reload ever started.
116 await until {
117 guard stub.seen.count == 2 else { return false }
118 if case .loading = model.state { return false }
119 return true
120 }
111 121
112 #expect(stub.seen.count == 2) 122 #expect(stub.seen.count == 2)
113 #expect(stub.seen[1].url.query()?.contains("argv=closed") == true) 123 #expect(stub.seen[1].url.query()?.contains("argv=closed") == true)
gitbayTests/SearchTests.swift +1 −1
@@ -137,7 +137,7 @@ struct SearchViewModelTests {
137 137
138 stub.enqueue(.init(status: 200, json: mixedResults)) 138 stub.enqueue(.init(status: 200, json: mixedResults))
139 model.kind = .issue 139 model.kind = .issue
140 try await Task.sleep(for: .milliseconds(150)) 140 await until { stub.seen.count == 2 }
141 141
142 #expect(argvFrom(try #require(stub.seen.last).url) 142 #expect(argvFrom(try #require(stub.seen.last).url)
143 == ["search", "parity", "--kind", "issue"]) 143 == ["search", "parity", "--kind", "issue"])
gitbayTests/SessionStoreTests.swift +3 −2
@@ -162,8 +162,9 @@ struct SessionStoreTests {
162 await #expect(throws: GitbayError.self) { 162 await #expect(throws: GitbayError.self) {
163 _ = try await client.readList(["repo", "list"], of: RepoSummary.self) 163 _ = try await client.readList(["repo", "list"], of: RepoSummary.self)
164 } 164 }
165 // The sign-out hops through the main actor; give it a beat. 165 // The sign-out hops through the main actor; wait for it rather
166 try await Task.sleep(for: .milliseconds(200)) 166 // than for a fixed interval to pass.
167 await until { session.current == nil }
167 168
168 #expect(session.current == nil) 169 #expect(session.current == nil)
169 #expect(session.client == nil) 170 #expect(session.client == nil)
gitbayTests/StubProtocol.swift +16
@@ -161,3 +161,19 @@ nonisolated final class Mutex<Value>: @unchecked Sendable {
161 return body(&value) 161 return body(&value)
162 } 162 }
163} 163}
164
165/// Wait for a condition rather than sleeping a fixed interval. The view
166/// models reload through unstructured Tasks, so a fixed sleep is a race:
167/// under load the reload has not landed when it expires. Polling is both
168/// faster in the common case and deterministic.
169@MainActor
170func until(
171 _ timeout: Duration = .seconds(2),
172 _ condition: () -> Bool
173) async {
174 let deadline = ContinuousClock.now + timeout
175 while !condition() {
176 guard ContinuousClock.now < deadline else { return }
177 try? await Task.sleep(for: .milliseconds(10))
178 }
179}