Browse Source

py38: cannot set_result more than once (#28700)

josh 3 years ago
parent
commit
505f7972f2
2 changed files with 23 additions and 12 deletions
  1. 4 4
      src/sentry/utils/concurrent.py
  2. 19 8
      tests/sentry/utils/test_concurrent.py

+ 4 - 4
src/sentry/utils/concurrent.py

@@ -92,10 +92,10 @@ class TimedFuture(Future):
 
     def set_result(self, *args, **kwargs):
         with self._condition:
-            # This method always overwrites the result, so we always overwrite
-            # the timing, even if another timing was already recorded.
-            self.__timing[1] = time()
-            return super().set_result(*args, **kwargs)
+            _time = time()
+            result = super().set_result(*args, **kwargs)
+            self.__timing[1] = _time
+            return result
 
     def set_exception(self, *args, **kwargs):
         with self._condition:

+ 19 - 8
tests/sentry/utils/test_concurrent.py

@@ -17,7 +17,6 @@ from sentry.utils.concurrent import (
 )
 
 
-@pytest.mark.skipif(sys.version_info[0] == 3, reason="TODO(python3): stalls on python3")
 def test_execute():
     assert execute(_thread.get_ident).result() != _thread.get_ident()
 
@@ -108,9 +107,25 @@ def test_timed_future_success():
         future.set_result(None)
         assert future.get_timing() == (1.0, 2.0)
 
-    with timestamp(3.0):
-        future.set_result(None)
-        assert future.get_timing() == (1.0, 3.0)
+
+@pytest.mark.skipif(sys.version_info[:2] < (3, 8), reason="doesn't apply to this python version")
+def test_time_is_not_overwritten_if_fail_to_set_result():
+    future = TimedFuture()
+
+    with timestamp(1.0):
+        future.set_running_or_notify_cancel()
+        future.set_result(1)
+        assert future.get_timing() == (1.0, 1.0)
+
+    from concurrent.futures import InvalidStateError
+
+    with timestamp(2.0):
+        try:
+            future.set_result(1)
+        except InvalidStateError:
+            pass
+        # If set_result fails, the time shouldn't be overwritten.
+        assert future.get_timing() == (1.0, 1.0)
 
 
 def test_timed_future_error():
@@ -125,10 +140,6 @@ def test_timed_future_error():
         future.set_exception(None)
         assert future.get_timing() == (1.0, 2.0)
 
-    with timestamp(3.0):
-        future.set_exception(None)
-        assert future.get_timing() == (1.0, 3.0)
-
 
 def test_timed_future_cancel():
     future = TimedFuture()