[6.2] Add some basic tests for Observations with nearly full coverage #82273
Conversation
|
@swift-ci please smoke test |
|
@swift-ci please test |
airspeedswift
changed the title
|
@swift-ci test macOS |
|
@swift-ci please test |
| } | ||
|
|
||
| @Observable | ||
| final class AsyncTestModel { |
There was a problem hiding this comment.
nit: this type doesn't appear to be used – is that expected?
|
|
||
| suite.test("Basic element emission") { | ||
| let model = TestModel() | ||
| var emissionCount = 0 |
There was a problem hiding this comment.
nit: this variable appears unused
| Task { | ||
| await Task.yield() | ||
| model.name = "updated" | ||
| model.value = 100 |
There was a problem hiding this comment.
since there's no synchronization, couldn't this result in 'tearing' given the ordering:
- set
model.name - resume L92
- invoke closure
- set
model.value
could this cause test flakes? or is @MainActor being inherited in some way here (if so, perhaps an isolation assertion would be worthwhile)?
| var emissionCount = 0 | ||
|
|
||
| let observations = Observations<Int, Never> { | ||
| model.value |
There was a problem hiding this comment.
tangential question: how is this building since the TestModel type isn't Sendable?
| expectEqual(result1.count, 3) | ||
| expectEqual(result2.count, 3) | ||
| expectEqual(result1[0], 0) // Initial value | ||
| expectEqual(result2[0], 0) // Initial value |
There was a problem hiding this comment.
perhaps this should this compare to [0, 100, 200] if the order is an expected invariant
| expectEqual(result2[0], 0) // Initial value | ||
| } | ||
|
|
||
| suite.test("Task cancellation behavior") { |
There was a problem hiding this comment.
have you by chance thought about this possible issue with cancellation?
|
@swift-ci please test macOS |
3 participants
This is a cherry pick of the mainline #82272