Skip to content

Commit

Permalink
Fix invalid logic with zip operator and premature termination.
Browse files Browse the repository at this point in the history
  • Loading branch information
kzaher committed Apr 15, 2019
1 parent ff9f4e0 commit f962ddd
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 38 deletions.
16 changes: 0 additions & 16 deletions RxSwift/Observables/Zip.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,6 @@ class ZipSink<O: ObserverType> : Sink<O>, ZipSinkProtocol {
self.dispose()
}
}
else {
var allOthersDone = true

let arity = self._isDone.count
for i in 0 ..< arity {
if i != index && !self._isDone[i] {
allOthersDone = false
break
}
}

if allOthersDone {
self.forwardOn(.completed)
self.dispose()
}
}
}

func fail(_ error: Swift.Error) {
Expand Down
2 changes: 1 addition & 1 deletion Tests/RxSwiftTests/Observable+CombineLatestTests+arity.tt
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ extension ObservableCombineLatestTest {
factory(<%= (Array(0..<i).map { "e\($0)" }).joined(separator: ", ") %>)
}

XCTAssertEqual(res.events, [completed(<%= 200 + 10 * i %>)])
XCTAssertEqual(res.events, [.completed(<%= 200 + 10 * i %>)])

<% for j in 0..<i { %>
XCTAssertEqual(e<%= j %>.subscriptions, [Subscription(200, <%= 200 + 10 * (j + 1) %>)])
Expand Down
28 changes: 14 additions & 14 deletions Tests/RxSwiftTests/Observable+ZipTests+arity.swift
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,12 @@ extension ObservableZipTest {

XCTAssertEqual(res.events, [
.next(210, 10),
.completed(220)
.completed(230)
])


XCTAssertEqual(e0.subscriptions, [Subscription(200, 220)])
XCTAssertEqual(e1.subscriptions, [Subscription(200, 220)])
XCTAssertEqual(e1.subscriptions, [Subscription(200, 230)])
}
}

Expand Down Expand Up @@ -419,13 +419,13 @@ extension ObservableZipTest {

XCTAssertEqual(res.events, [
.next(210, 15),
.completed(230)
.completed(240)
])


XCTAssertEqual(e0.subscriptions, [Subscription(200, 220)])
XCTAssertEqual(e1.subscriptions, [Subscription(200, 230)])
XCTAssertEqual(e2.subscriptions, [Subscription(200, 230)])
XCTAssertEqual(e2.subscriptions, [Subscription(200, 240)])
}
}

Expand Down Expand Up @@ -670,14 +670,14 @@ extension ObservableZipTest {

XCTAssertEqual(res.events, [
.next(210, 20),
.completed(240)
.completed(250)
])


XCTAssertEqual(e0.subscriptions, [Subscription(200, 220)])
XCTAssertEqual(e1.subscriptions, [Subscription(200, 230)])
XCTAssertEqual(e2.subscriptions, [Subscription(200, 240)])
XCTAssertEqual(e3.subscriptions, [Subscription(200, 240)])
XCTAssertEqual(e3.subscriptions, [Subscription(200, 250)])
}
}

Expand Down Expand Up @@ -951,15 +951,15 @@ extension ObservableZipTest {

XCTAssertEqual(res.events, [
.next(210, 25),
.completed(250)
.completed(260)
])


XCTAssertEqual(e0.subscriptions, [Subscription(200, 220)])
XCTAssertEqual(e1.subscriptions, [Subscription(200, 230)])
XCTAssertEqual(e2.subscriptions, [Subscription(200, 240)])
XCTAssertEqual(e3.subscriptions, [Subscription(200, 250)])
XCTAssertEqual(e4.subscriptions, [Subscription(200, 250)])
XCTAssertEqual(e4.subscriptions, [Subscription(200, 260)])
}
}

Expand Down Expand Up @@ -1263,7 +1263,7 @@ extension ObservableZipTest {

XCTAssertEqual(res.events, [
.next(210, 30),
.completed(260)
.completed(270)
])


Expand All @@ -1272,7 +1272,7 @@ extension ObservableZipTest {
XCTAssertEqual(e2.subscriptions, [Subscription(200, 240)])
XCTAssertEqual(e3.subscriptions, [Subscription(200, 250)])
XCTAssertEqual(e4.subscriptions, [Subscription(200, 260)])
XCTAssertEqual(e5.subscriptions, [Subscription(200, 260)])
XCTAssertEqual(e5.subscriptions, [Subscription(200, 270)])
}
}

Expand Down Expand Up @@ -1607,7 +1607,7 @@ extension ObservableZipTest {

XCTAssertEqual(res.events, [
.next(210, 35),
.completed(270)
.completed(280)
])


Expand All @@ -1617,7 +1617,7 @@ extension ObservableZipTest {
XCTAssertEqual(e3.subscriptions, [Subscription(200, 250)])
XCTAssertEqual(e4.subscriptions, [Subscription(200, 260)])
XCTAssertEqual(e5.subscriptions, [Subscription(200, 270)])
XCTAssertEqual(e6.subscriptions, [Subscription(200, 270)])
XCTAssertEqual(e6.subscriptions, [Subscription(200, 280)])
}
}

Expand Down Expand Up @@ -1984,7 +1984,7 @@ extension ObservableZipTest {

XCTAssertEqual(res.events, [
.next(210, 40),
.completed(280)
.completed(290)
])


Expand All @@ -1995,7 +1995,7 @@ extension ObservableZipTest {
XCTAssertEqual(e4.subscriptions, [Subscription(200, 260)])
XCTAssertEqual(e5.subscriptions, [Subscription(200, 270)])
XCTAssertEqual(e6.subscriptions, [Subscription(200, 280)])
XCTAssertEqual(e7.subscriptions, [Subscription(200, 280)])
XCTAssertEqual(e7.subscriptions, [Subscription(200, 290)])
}
}

Expand Down
5 changes: 2 additions & 3 deletions Tests/RxSwiftTests/Observable+ZipTests+arity.tt
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,11 @@ extension ObservableZipTest {

XCTAssertEqual(res.events, [
.next(210, <%= 5 * i %>),
.completed(<%= 220 + (i - 2) * 10 %>)
.completed(<%= 220 + (i - 1) * 10 %>)
])

<% for j in 0..<i-1 { %>
<% for j in 0..<i { %>
XCTAssertEqual(e<%= j %>.subscriptions, [Subscription(200, <%= 220 + 10 * j %>)])<% } %>
XCTAssertEqual(e<%= i-1 %>.subscriptions, [Subscription(200, <%= 220 + 10 * (i - 2) %>)])
}
}

Expand Down
8 changes: 4 additions & 4 deletions Tests/RxSwiftTests/Observable+ZipTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ extension ObservableZipTest {
}

let messages = [
Recorded.completed(215, Int.self)
Recorded.completed(220, Int.self)
]
XCTAssertEqual(res.events, messages)

Expand All @@ -96,7 +96,7 @@ extension ObservableZipTest {
])

XCTAssertEqual(o.subscriptions, [
Subscription(200, 215)
Subscription(200, 220)
])
}

Expand All @@ -119,7 +119,7 @@ extension ObservableZipTest {
}

let messages = [
Recorded.completed(215, Int.self)
Recorded.completed(220, Int.self)
]
XCTAssertEqual(res.events, messages)

Expand All @@ -128,7 +128,7 @@ extension ObservableZipTest {
])

XCTAssertEqual(o.subscriptions, [
Subscription(200, 215)
Subscription(200, 220)
])
}

Expand Down

0 comments on commit f962ddd

Please sign in to comment.