From 8f2c372b823962c7e9597c3f763d9c4020cdb516 Mon Sep 17 00:00:00 2001
From: Idel Pivnitskiy <idel.pivnitskiy@apple.com>
Date: Mon, 28 Jun 2021 11:49:11 -0500
Subject: [PATCH] `TestSubscription#requestedEquals(0)` incorrectly validates
 the value (#1642)

Motivation:

`TestSubscription#requestedEquals` depends on the passed `value` and if
`value == 0` it does not check the current cumulative value of requested
elements, it only checks if `request(long)` was ever invoked or not.

Modifications:

- Fix validation in `TestSubscription#requestedEquals`;
- Adjust existing tests to account for new behavior;

Result:

`TestSubscription#requestedEquals(0)` validates that `request(long)` was
invoked and the current cumulative value if equal to `0`.
---
 .../completable/CompletableConcatWithPublisherTest.java    | 7 +++----
 .../api/single/SingleConcatWithPublisherTest.java          | 2 +-
 .../io/servicetalk/concurrent/api/TestSubscription.java    | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/servicetalk-concurrent-api/src/test/java/io/servicetalk/concurrent/api/completable/CompletableConcatWithPublisherTest.java b/servicetalk-concurrent-api/src/test/java/io/servicetalk/concurrent/api/completable/CompletableConcatWithPublisherTest.java
index 15d3b21e86..de83733231 100644
--- a/servicetalk-concurrent-api/src/test/java/io/servicetalk/concurrent/api/completable/CompletableConcatWithPublisherTest.java
+++ b/servicetalk-concurrent-api/src/test/java/io/servicetalk/concurrent/api/completable/CompletableConcatWithPublisherTest.java
@@ -110,16 +110,15 @@ void invalidThenValidRequest() {
     @Test
     void request0Propagated() {
         subscriber.awaitSubscription().request(0);
-        triggerNextSubscribe();
-        assertThat("Invalid request-n not propagated " + subscription, subscription.requestedEquals(0),
+        triggerNextSubscribe(); // If subscribe happens after request(0) it will be mapped into -1
+        assertThat("Invalid request-n not propagated " + subscription, subscription.requestedEquals(-1),
                 is(true));
     }
 
     @Test
     void request0PropagatedAfterComplete() {
-        source.onComplete();
+        triggerNextSubscribe();
         subscriber.awaitSubscription().request(0);
-        next.onSubscribe(subscription);
         assertThat("Invalid request-n not propagated " + subscription, subscription.requestedEquals(0),
                 is(true));
     }
diff --git a/servicetalk-concurrent-api/src/test/java/io/servicetalk/concurrent/api/single/SingleConcatWithPublisherTest.java b/servicetalk-concurrent-api/src/test/java/io/servicetalk/concurrent/api/single/SingleConcatWithPublisherTest.java
index fb22590240..c01b6ffe31 100644
--- a/servicetalk-concurrent-api/src/test/java/io/servicetalk/concurrent/api/single/SingleConcatWithPublisherTest.java
+++ b/servicetalk-concurrent-api/src/test/java/io/servicetalk/concurrent/api/single/SingleConcatWithPublisherTest.java
@@ -149,8 +149,8 @@ private void invalidThenValidRequest(long invalidN) {
     void request0PropagatedAfterSuccess() {
         source.onSuccess(1);
         subscriber.awaitSubscription().request(1); // get the success from the Single
-        subscriber.awaitSubscription().request(0);
         next.onSubscribe(subscription);
+        subscriber.awaitSubscription().request(0);
         assertThat("Invalid request-n propagated " + subscription, subscription.requestedEquals(0),
                 is(true));
     }
diff --git a/servicetalk-concurrent-api/src/testFixtures/java/io/servicetalk/concurrent/api/TestSubscription.java b/servicetalk-concurrent-api/src/testFixtures/java/io/servicetalk/concurrent/api/TestSubscription.java
index a3d0a2a0b3..859afe5f12 100644
--- a/servicetalk-concurrent-api/src/testFixtures/java/io/servicetalk/concurrent/api/TestSubscription.java
+++ b/servicetalk-concurrent-api/src/testFixtures/java/io/servicetalk/concurrent/api/TestSubscription.java
@@ -60,7 +60,7 @@ public long requested() {
      * @return {@code true} if the cumulative value of {@link #request(long)} matches {@code value}.
      */
     public boolean requestedEquals(long value) {
-        return value == 0 && requestCalled || value != 0 && requested.get() == value;
+        return (value != 0 || requestCalled) && requested.get() == value;
     }
 
     /**