From 5d8e366dc3aa8ccb7a9dcbf60dc4230e8baf4893 Mon Sep 17 00:00:00 2001 From: brunoais Date: Tue, 11 Dec 2018 16:15:48 +0000 Subject: [PATCH 1/4] [bug]StreamingPullFuture init with _cancelled = True --- pubsub/google/cloud/pubsub_v1/subscriber/futures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pubsub/google/cloud/pubsub_v1/subscriber/futures.py b/pubsub/google/cloud/pubsub_v1/subscriber/futures.py index 11fddf24abd0..f3c06416083b 100644 --- a/pubsub/google/cloud/pubsub_v1/subscriber/futures.py +++ b/pubsub/google/cloud/pubsub_v1/subscriber/futures.py @@ -30,7 +30,7 @@ def __init__(self, manager): super(StreamingPullFuture, self).__init__() self._manager = manager self._manager.add_close_callback(self._on_close_callback) - self._cancelled = True + self._cancelled = False def _on_close_callback(self, manager, result): if result is None: From d9670513d1ceddcd893aa9ee91c97ed9ba4f2c4d Mon Sep 17 00:00:00 2001 From: brunoais Date: Tue, 11 Dec 2018 19:31:25 +0000 Subject: [PATCH 2/4] add tests to confirm future starts with cancelled as false --- pubsub/tests/unit/pubsub_v1/test_futures.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pubsub/tests/unit/pubsub_v1/test_futures.py b/pubsub/tests/unit/pubsub_v1/test_futures.py index 11349d5d480a..414ea8614a30 100644 --- a/pubsub/tests/unit/pubsub_v1/test_futures.py +++ b/pubsub/tests/unit/pubsub_v1/test_futures.py @@ -33,6 +33,7 @@ def test_constructor_defaults(): assert future._exception == futures.Future._SENTINEL assert future._callbacks == [] assert future._completed is Event.return_value + assert future._cancelled is False Event.assert_called_once_with() @@ -45,6 +46,7 @@ def test_constructor_explicit_completed(): assert future._exception == futures.Future._SENTINEL assert future._callbacks == [] assert future._completed is completed + assert future._cancelled is False def test_cancel(): From 0501c102a8f6aa232e65f7a0fe48205d90cdc281 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 12 Dec 2018 13:10:30 -0500 Subject: [PATCH 3/4] Remove ill-directed assertions. I wrongly asked that they be added in this file, rather than in `pubsub/tests/unit/pubsub_v1/subscriber/test_futures_subscriber.py` --- pubsub/tests/unit/pubsub_v1/test_futures.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pubsub/tests/unit/pubsub_v1/test_futures.py b/pubsub/tests/unit/pubsub_v1/test_futures.py index 414ea8614a30..11349d5d480a 100644 --- a/pubsub/tests/unit/pubsub_v1/test_futures.py +++ b/pubsub/tests/unit/pubsub_v1/test_futures.py @@ -33,7 +33,6 @@ def test_constructor_defaults(): assert future._exception == futures.Future._SENTINEL assert future._callbacks == [] assert future._completed is Event.return_value - assert future._cancelled is False Event.assert_called_once_with() @@ -46,7 +45,6 @@ def test_constructor_explicit_completed(): assert future._exception == futures.Future._SENTINEL assert future._callbacks == [] assert future._completed is completed - assert future._cancelled is False def test_cancel(): From b32976ae2de23fe2a4d3a865f2cf21959754f040 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 12 Dec 2018 13:12:35 -0500 Subject: [PATCH 4/4] Add assertion in the correct spot. --- .../tests/unit/pubsub_v1/subscriber/test_futures_subscriber.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pubsub/tests/unit/pubsub_v1/subscriber/test_futures_subscriber.py b/pubsub/tests/unit/pubsub_v1/subscriber/test_futures_subscriber.py index 4d41713e6ec8..2b4566018f7f 100644 --- a/pubsub/tests/unit/pubsub_v1/subscriber/test_futures_subscriber.py +++ b/pubsub/tests/unit/pubsub_v1/subscriber/test_futures_subscriber.py @@ -34,6 +34,7 @@ def test_default_state(self): assert future.running() assert not future.done() + assert not future.cancelled() future._manager.add_close_callback.assert_called_once_with( future._on_close_callback )