fixes for take_until tests

Summary:
there were test failures that fell into three buckets

first, the strands worker had concurrency issues. the handoff between submit and the worker was wrong.

second, take_until was not waiting for the producer to signal done. this was originally by-design. now the expectation is that take until will not early-complete on cancellation.

third, the tests had concurrency issues. the EXPECT assertions were not allowing for non-deterministic interleaving.

Reviewed By: lewissbaker

Differential Revision: D14947473

fbshipit-source-id: 36f7cf87b9226367dc44afed663fa6418503c059

folly/experimental/pushmi/o/extension_operators.h

@@ -144,7 +144,7 @@ struct set_value_fn {
     std::tuple<VN...> vn_;
     PUSHMI_TEMPLATE(class Out)
     (requires ReceiveValue<Out, VN...>) //
-    void operator()(Out out) {
+    void operator()(Out& out) {
       ::folly::pushmi::apply(
           ::folly::pushmi::set_value,
           std::tuple_cat(std::tuple<Out>{std::move(out)}, std::move(vn_)));
@@ -165,7 +165,7 @@ struct set_error_fn {
     E e_;
     PUSHMI_TEMPLATE(class Out)
     (requires ReceiveError<Out, E>) //
-    void operator()(Out out) {
+    void operator()(Out& out) {
       set_error(out, std::move(e_));
     }
   };
@@ -183,7 +183,7 @@ struct set_done_fn {
   struct impl {
     PUSHMI_TEMPLATE(class Out)
     (requires Receiver<Out>) //
-    void operator()(Out out) {
+    void operator()(Out& out) {
       set_done(out);
     }
   };
@@ -201,7 +201,7 @@ struct set_starting_fn {
     Up up_;
     PUSHMI_TEMPLATE(class Out)
     (requires Receiver<Out>) //
-    void operator()(Out out) {
+    void operator()(Out& out) {
       set_starting(out, std::move(up_));
     }
   };
@@ -308,7 +308,7 @@ struct now_fn {
   struct impl {
     PUSHMI_TEMPLATE(class In)
     (requires TimeExecutor<std::decay_t<In>>) //
-    auto operator()(In&& in) const {
+    auto operator()(In& in) const {
       return ::folly::pushmi::now(in);
     }
   };

folly/experimental/pushmi/o/from.h

@@ -130,8 +130,8 @@ struct flow_from_up {
       make_receiver([p = p_, requested](auto) {
         auto remaining = requested;
         // this loop is structured to work when there is
-        // re-entrancy out.value in the loop may call up.value.
-        // to handle this the state of p->c must be captured and
+        // re-entrancy. out.value in the loop may call up.value.
+        // to handle this, the state of p->c must be captured and
         // the remaining and p->c must be changed before
         // out.value is called.
         while (remaining-- > 0 && !p->stop && p->c != p->end) {

folly/experimental/pushmi/test/o/TakeUntilTest.cpp

@@ -57,7 +57,9 @@ struct ReceiverSignals_ : Base {
   ReceiverSignals_& operator=(const ReceiverSignals_&) = default;
   ReceiverSignals_(ReceiverSignals_&&) = default;
   ReceiverSignals_& operator=(ReceiverSignals_&&) = default;
-  explicit ReceiverSignals_(std::string id) : id_(std::move(id)), counters_(std::make_shared<receiver_counters>()) {}
+  explicit ReceiverSignals_(std::string id) :
+    id_(std::move(id)),
+    counters_(std::make_shared<receiver_counters>()) {}
   std::string id_;
   std::shared_ptr<receiver_counters> counters_;
 
@@ -108,6 +110,12 @@ struct ReceiverSignals_ : Base {
     }
   }
 
+  template<class Fn>
+  void verifyValues(Fn fn) {
+    EXPECT_THAT(fn(counters_->values_.load()), Eq(true))
+        << "[" << id_
+        << "]::verifyValues() expected the value signal(s) to satisfy the predicate.";
+  }
   void verifyValues(int count) {
     EXPECT_THAT(counters_->values_.load(), Eq(count))
         << "[" << id_
@@ -159,6 +167,10 @@ using ReceiverSignals =
 using FlowReceiverSignals =
     detail::ReceiverSignals_<mi::flow_receiver<>>;
 
+auto zeroOrOne = [](int count){
+  return count == 0 || count == 1;
+};
+
 TEST(EmptySourceEmptyTriggerTrampoline, TakeUntil) {
   std::array<int, 0> ae{};
   auto e = op::flow_from(ae, mi::trampolines);
@@ -237,7 +249,7 @@ TEST(EmptySourceValueTrigger, TakeUntil) {
   source.verifyFinal();
 
   trigger.wait();
-  trigger.verifyValues(1);
+  trigger.verifyValues(zeroOrOne);
   trigger.verifyDones();
   trigger.verifyFinal();
 
@@ -263,6 +275,7 @@ TEST(ValueSourceEmptyTrigger, TakeUntil) {
       op::for_each(each);
 
   source.wait();
+  source.verifyValues(zeroOrOne);
   source.verifyDones();
   source.verifyFinal();
 
@@ -272,7 +285,7 @@ TEST(ValueSourceEmptyTrigger, TakeUntil) {
   trigger.verifyFinal();
 
   each.wait();
-  each.verifyValues(0);
+  each.verifyValues(zeroOrOne);
   each.verifyDones();
   each.verifyFinal();
 }
@@ -291,16 +304,17 @@ TEST(ValueSourceValueTrigger, TakeUntil) {
       op::for_each(each);
 
   source.wait();
+  source.verifyValues(zeroOrOne);
   source.verifyDones();
   source.verifyFinal();
 
   trigger.wait();
-  trigger.verifyValues(1);
+  trigger.verifyValues(zeroOrOne);
   trigger.verifyDones();
   trigger.verifyFinal();
 
   each.wait();
-  each.verifyValues(0);
+  each.verifyValues(zeroOrOne);
   each.verifyDones();
   each.verifyFinal();
 }