Address review: use fut.exception() result, drop the redundant handling, test the timeout case, remove NEWS

Author: tonghuaroot

Lib/concurrent/futures/_base.py

@@ -309,21 +309,14 @@ def wait(fs, timeout=None, return_when=ALL_COMPLETED):
 def _result_or_cancel(fut, timeout=None):
     try:
         try:
-            if timeout is not None:
-                # Wait out the timeout separately from retrieving the result, so
-                # that a TimeoutError raised by the call is not mistaken for the
-                # map() timeout.  Future.exception() raises TimeoutError only
-                # when the wait itself times out, never for the call's own.
-                try:
-                    fut.exception(timeout)
-                except TimeoutError:
-                    raise
-                except CancelledError:
-                    pass
-            try:
-                return (fut.result(), None)
-            except BaseException as exc:
+            # fut.exception() returns the call's own error but raises
+            # TimeoutError only for a map() timeout.
+            exc = fut.exception(timeout)
+            if exc is not None:
                 return (None, exc)
+            return (fut.result(), None)
+        except CancelledError as exc:
+            return (None, exc)
         finally:
             fut.cancel()
     finally:

Lib/test/test_concurrent_futures/executor.py

@@ -29,7 +29,6 @@ def raiser(exception, msg='std'):
     raise exception(msg)
 
 
-# Used in test_map_timeout_from_callable
 def timeout_on_one(x):
     if x == 1:
         raise TimeoutError
@@ -96,13 +95,17 @@ def test_map_exception(self):
 
     @warnings_helper.ignore_fork_in_thread_deprecation_warnings()
     def test_map_timeout_from_callable(self):
-        # A TimeoutError from the callable is not the map() timeout.
-        i = self.executor.map(timeout_on_one, [0, 1, 2, 3])
-        self.assertEqual(next(i), 0)
-        self.assertRaises(TimeoutError, next, i)
-        self.assertEqual(next(i), 2)
-        self.assertEqual(next(i), 3)
-        self.assertRaises(StopIteration, next, i)
+        # A TimeoutError from the callable is not the map() timeout, whether
+        # or not a map() timeout is set.
+        for timeout in (None, support.SHORT_TIMEOUT):
+            with self.subTest(timeout=timeout):
+                i = self.executor.map(timeout_on_one, [0, 1, 2, 3],
+                                      timeout=timeout)
+                self.assertEqual(next(i), 0)
+                self.assertRaises(TimeoutError, next, i)
+                self.assertEqual(next(i), 2)
+                self.assertEqual(next(i), 3)
+                self.assertRaises(StopIteration, next, i)
 
     @warnings_helper.ignore_fork_in_thread_deprecation_warnings()
     @support.requires_resource('walltime')