git-cl: Fix get_cl_statuses for Python 3 and add tests.

In Python 3 the semantics of `raise StopIteration` inside a generator
function changed.

Bug: 1002209
Change-Id: I51222a5006c4024b3a6a06d344423ee36870825a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2071056
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

git_cl.py

@@ -10,7 +10,6 @@
 from __future__ import print_function
 
 from distutils.version import LooseVersion
-from multiprocessing.pool import ThreadPool
 import base64
 import collections
 import datetime
@@ -3418,7 +3417,7 @@ def get_cl_statuses(changes, fine_grained, max_processes=None):
   See GetStatus() for a list of possible statuses.
   """
   if not changes:
-    raise StopIteration()
+    return
 
   if not fine_grained:
     # Fast path which doesn't involve querying codereview servers.
@@ -3445,14 +3444,14 @@ def get_cl_statuses(changes, fine_grained, max_processes=None):
     threads_count = max(1, min(threads_count, max_processes))
   logging.debug('querying %d CLs using %d threads', len(changes), threads_count)
 
-  pool = ThreadPool(threads_count)
+  pool = multiprocessing.pool.ThreadPool(threads_count)
   fetched_cls = set()
   try:
     it = pool.imap_unordered(fetch, changes).__iter__()
     while True:
       try:
         cl, status = it.next(timeout=5)
-      except multiprocessing.TimeoutError:
+      except (multiprocessing.TimeoutError, StopIteration):
         break
       fetched_cls.add(cl)
       yield cl, status

tests/git_cl_test.py

@@ -12,6 +12,7 @@ from __future__ import unicode_literals
 import datetime
 import json
 import logging
+import multiprocessing
 import os
 import pprint
 import shutil
@@ -194,6 +195,47 @@ class TestGitClBasic(unittest.TestCase):
     cl.has_description = True
     self.assertEqual(cl.FetchDescription(), 'x')
 
+  @mock.patch('git_cl.Changelist.EnsureAuthenticated')
+  @mock.patch('git_cl.Changelist.GetStatus', lambda cl: cl.status)
+  def test_get_cl_statuses(self, *_mocks):
+    statuses = [
+        'closed', 'commit', 'dry-run', 'lgtm', 'reply', 'unsent', 'waiting']
+    changes = []
+    for status in statuses:
+      cl = git_cl.Changelist()
+      cl.status = status
+      changes.append(cl)
+
+    actual = set(git_cl.get_cl_statuses(changes, True))
+    self.assertEqual(set(zip(changes, statuses)), actual)
+
+  def test_get_cl_statuses_no_changes(self):
+    self.assertEqual([], list(git_cl.get_cl_statuses([], True)))
+
+  @mock.patch('git_cl.Changelist.EnsureAuthenticated')
+  @mock.patch('multiprocessing.pool.ThreadPool')
+  def test_get_cl_statuses_timeout(self, *_mocks):
+    changes = [git_cl.Changelist() for _ in range(2)]
+    pool = multiprocessing.pool.ThreadPool()
+    it = pool.imap_unordered.return_value.__iter__ = mock.Mock()
+    it.return_value.next.side_effect = [
+        (changes[0], 'lgtm'),
+        multiprocessing.TimeoutError,
+    ]
+
+    actual = list(git_cl.get_cl_statuses(changes, True))
+    self.assertEqual([(changes[0], 'lgtm'), (changes[1], 'error')], actual)
+
+  @mock.patch('git_cl.Changelist.GetIssueURL')
+  def test_get_cl_statuses_not_finegrained(self, _mock):
+    changes = [git_cl.Changelist() for _ in range(2)]
+    urls = ['some-url', None]
+    git_cl.Changelist.GetIssueURL.side_effect = urls
+
+    actual = set(git_cl.get_cl_statuses(changes, False))
+    self.assertEqual(
+        set([(changes[0], 'waiting'), (changes[1], 'error')]), actual)
+
   def test_set_preserve_tryjobs(self):
     d = git_cl.ChangeDescription('Simple.')
     d.set_preserve_tryjobs()