Refactor eclipse/project.py to make it testable

Move all the logic of tools/eclipse/project.py so it can be tested and
add a few tests.

Add a py_test target so one can run the tests from bazel using:

  bazel test --test_output=streamed //tools/eclipse:test

Add a bunch of tests and a helper to generate a coverage report. The
resulting coverage report can be checked by opening
`tools/eclipse/htmlcov/project_py.html`.

Change-Id: I03582d70a5ec28117ab6d3b4c098d84e12e3d8dc
diff --git a/tools/eclipse/.gitignore b/tools/eclipse/.gitignore
new file mode 100644
index 0000000..1bc5aa1
--- /dev/null
+++ b/tools/eclipse/.gitignore
@@ -0,0 +1 @@
+/.coverage
diff --git a/tools/eclipse/BUILD b/tools/eclipse/BUILD
index 789a607..451f22d 100644
--- a/tools/eclipse/BUILD
+++ b/tools/eclipse/BUILD
@@ -1,4 +1,4 @@
-load("@rules_python//python:defs.bzl", "py_binary")
+load("@rules_python//python:defs.bzl", "py_binary", "py_test")
 
 package(
     default_visibility = ["//visibility:public"],
@@ -9,3 +9,16 @@
     srcs = ["project.py"],
     main = "project.py",
 )
+
+py_test(
+    name = "test_project",
+    srcs = ["test_project.py"],
+    deps = [":project"],
+)
+
+test_suite(
+    name = "test",
+    tests = [
+        "test_project",
+    ]
+)
diff --git a/tools/eclipse/project.py b/tools/eclipse/project.py
index a24266f..93eeb3a 100755
--- a/tools/eclipse/project.py
+++ b/tools/eclipse/project.py
@@ -28,77 +28,98 @@
   'JavaSE-11',
 ])
 
-opts = argparse.ArgumentParser("Create Eclipse Project")
-opts.add_argument('-r', '--root', help='Root directory entry', required=True)
-opts.add_argument('-n', '--name', help='Project name')
-opts.add_argument('-x', '--exclude', action='append', help='Exclude paths')
-opts.add_argument('-b', '--batch', action='store_true',
-                  dest='batch', help='Bazel batch option')
-opts.add_argument('--bazel',
-                  help=('name of the bazel executable. Defaults to using'
-                        ' bazelisk if found, or bazel if bazelisk is not'
-                        ' found.'),
-                  action='store', default=None, dest='bazel_exe')
-args = opts.parse_args()
+class EclipseProject():
 
-root = args.root
-ROOT = os.path.abspath(root)
-while not os.path.exists(os.path.join(ROOT, 'WORKSPACE')):
-  if ROOT == '/':
-    print('Could not find root of project: no WORKSPACE file found',
-          file=sys.stderr)
-    sys.exit(1)
-  ROOT = os.path.dirname(ROOT)
+  # Path to the Bazel/Bazelisk binary
+  bazel_exe = None
+  # Root of the Bazel project (holding WORKSPACE file)
+  ROOT = None
 
-batch_option = '--batch' if args.batch else None
+  def main(self):
+    self.bazel_exe = self.find_bazel(self.args.bazel_exe)
 
-def find_bazel():
-  if args.bazel_exe:
+    self.ROOT = self.find_root(self.args.root)
+
+    project_name = (self.args.name if self.args.name
+                    else os.path.basename(self.ROOT))
+
+    self.gen_project(project_name, root=self.ROOT)
+    self.gen_classpath(self.retrieve_ext_location().decode('utf-8'))
+
+  def _get_argument_parser(self):
+    opts = argparse.ArgumentParser("Create Eclipse Project")
+    opts.add_argument('-r', '--root', help='Root directory entry',
+                      required=True)
+    opts.add_argument('-n', '--name', help='Project name')
+    opts.add_argument('-x', '--exclude', action='append', help='Exclude paths')
+    opts.add_argument('-b', '--batch', action='store_true',
+                      dest='batch', help='Bazel batch option')
+    opts.add_argument('--bazel',
+                      help=('name of the bazel executable. Defaults to using'
+                            ' bazelisk if found, or bazel if bazelisk is not'
+                            ' found.'),
+                      action='store', default=None, dest='bazel_exe')
+    return opts
+
+  def parse_args(self, args):
+    self.args = self._get_argument_parser().parse_args(args)
+    return self.args
+
+  def find_root(self, root):
+    ROOT = os.path.abspath(root)
+    while not os.path.exists(os.path.join(ROOT, 'WORKSPACE')):
+      if ROOT == '/':
+        raise Exception(
+          'Could not find root of project: no WORKSPACE file found')
+      ROOT = os.path.dirname(ROOT)
+    return ROOT
+
+  def find_bazel(self, bazel_exe=None):
+    if bazel_exe:
+      try:
+        return subprocess.check_output(
+          ['which', bazel_exe]).strip().decode('UTF-8')
+      except subprocess.CalledProcessError:
+        raise Exception('Bazel command: %s not found' % bazel_exe)
     try:
       return subprocess.check_output(
-        ['which', args.bazel_exe]).strip().decode('UTF-8')
+        ['which', 'bazelisk']).strip().decode('UTF-8')
     except subprocess.CalledProcessError:
-      print('Bazel command: %s not found' % args.bazel_exe, file=sys.stderr)
-      sys.exit(1)
-  try:
+      try:
+        return subprocess.check_output(
+          ['which', 'bazel']).strip().decode('UTF-8')
+      except subprocess.CalledProcessError:
+        raise Exception(
+          "Neither bazelisk nor bazel found. Please see"
+          " Documentation/dev-bazel for instructions on installing"
+          " one of them.")
+
+  def _build_bazel_cmd(self, *args):
+    cmd = [self.bazel_exe]
+    if self.args.batch:
+      cmd.append('--batch')
+    for arg in args:
+      cmd.append(arg)
+    return cmd
+
+  def retrieve_ext_location(self):
     return subprocess.check_output(
-      ['which', 'bazelisk']).strip().decode('UTF-8')
-  except subprocess.CalledProcessError:
+      self._build_bazel_cmd('info', 'output_base')).strip()
+
+  def _query_classpath(self):
+    t = '//tools/eclipse:main_classpath_collect'
     try:
-      return subprocess.check_output(
-        ['which', 'bazel']).strip().decode('UTF-8')
+      cmd = self._build_bazel_cmd('build', t)
+      subprocess.check_call(cmd)
     except subprocess.CalledProcessError:
-      print("Neither bazelisk nor bazel found. Please see"
-            " Documentation/dev-bazel for instructions on installing"
-            " one of them.")
-      sys.exit(1)
+      raise Exception("Could not query classpath with:" % cmd)
+    name = 'bazel-bin/tools/eclipse/' + t.split(':')[1] + '.runtime_classpath'
+    return [line.rstrip('\n') for line in open(name)]
 
-bazel_exe = find_bazel()
-
-def _build_bazel_cmd(*args):
-  cmd = [bazel_exe]
-  if batch_option:
-    cmd.append('--batch')
-  for arg in args:
-    cmd.append(arg)
-  return cmd
-
-def retrieve_ext_location():
-  return subprocess.check_output(_build_bazel_cmd('info', 'output_base')).strip()
-
-def _query_classpath():
-  t = '//tools/eclipse:main_classpath_collect'
-  try:
-    subprocess.check_call(_build_bazel_cmd('build', t))
-  except subprocess.CalledProcessError:
-    exit(1)
-  name = 'bazel-bin/tools/eclipse/' + t.split(':')[1] + '.runtime_classpath'
-  return [line.rstrip('\n') for line in open(name)]
-
-def gen_project(name, root=ROOT):
-  p = os.path.join(root, '.project')
-  with open(p, 'w') as fd:
-    print("""\
+  def gen_project(name, root):
+    p = os.path.join(root, '.project')
+    with open(p, 'w') as fd:
+      print("""\
 <?xml version="1.0" encoding="UTF-8"?>
 <projectDescription>
   <name>%(name)s</name>
@@ -113,107 +134,111 @@
 </projectDescription>\
     """ % {"name": name}, file=fd)
 
-def gen_classpath(ext):
-  def make_classpath():
-    impl = xml.dom.minidom.getDOMImplementation()
-    return impl.createDocument(None, 'classpath', None)
+  def gen_classpath(self, ext):
+    def make_classpath():
+      impl = xml.dom.minidom.getDOMImplementation()
+      return impl.createDocument(None, 'classpath', None)
 
-  def classpathentry(kind, path, src=None, out=None, exported=None):
-    e = doc.createElement('classpathentry')
-    e.setAttribute('kind', kind)
-    # TODO(davido): Remove this and other exclude BUILD files hack
-    # when this Bazel bug is fixed:
-    # https://github.com/bazelbuild/bazel/issues/1083
-    if kind == 'src':
-      e.setAttribute('excluding', '**/BUILD')
-    e.setAttribute('path', path)
-    if src:
-      e.setAttribute('sourcepath', src)
-    if out:
-      e.setAttribute('output', out)
-    if exported:
-      e.setAttribute('exported', 'true')
-    doc.documentElement.appendChild(e)
-
-  doc = make_classpath()
-  src = set()
-  lib = set()
-
-  java_library = re.compile('bazel-out/(?:.*)-fastbuild/bin(.*)/[^/]+[.]jar$')
-  srcs = re.compile('(.*/external/[^/]+)/jar/(.*)[.]jar')
-  for p in _query_classpath():
-    m = java_library.match(p)
-    if m:
-      src.add(m.group(1).lstrip('/'))
-    else:
-      if ext is not None and p.startswith("external"):
-        p = os.path.join(ext, p)
-        lib.add(p)
-
-  src_paths = {}
-  for s in sorted(src):
-    out = None
-
-    if s.startswith('lib/'):
-      out = 'eclipse-out/lib'
-
-    p = os.path.join(s, 'java')
-    if os.path.exists(p):
-      classpathentry('src', p, out=out)
-      continue
-
-    for env in ['main', 'test', 'java', 'javatests']:
-      o = None
+    def classpathentry(kind, path, src=None, out=None, exported=None):
+      e = doc.createElement('classpathentry')
+      e.setAttribute('kind', kind)
+      # TODO(davido): Remove this and other exclude BUILD files hack
+      # when this Bazel bug is fixed:
+      # https://github.com/bazelbuild/bazel/issues/1083
+      if kind == 'src':
+        e.setAttribute('excluding', '**/BUILD')
+      e.setAttribute('path', path)
+      if src:
+        e.setAttribute('sourcepath', src)
       if out:
-        o = out + '/' + env
-      elif env == 'test' or env == 'javatests':
-        o = 'eclipse-out/test'
+        e.setAttribute('output', out)
+      if exported:
+        e.setAttribute('exported', 'true')
+      doc.documentElement.appendChild(e)
 
-      if s.startswith(env + '/'):
-        src_paths[env] = o
-        continue
+    doc = make_classpath()
+    src = set()
+    lib = set()
 
-      for srctype in ['java', 'resources']:
-        p = os.path.join(s, 'src', env, srctype)
-        if os.path.exists(p):
-          src_paths[p] = o
-
-  for s in src_paths:
-    classpathentry('src', s, out=src_paths[s])
-
-  for libs in [lib]:
-    for j in sorted(libs):
-      if excluded(j):
-        continue
-      s = None
-      m = srcs.match(j)
+    java_library = re.compile('bazel-out/(?:.*)-fastbuild/bin(.*)/[^/]+[.]jar$')
+    srcs = re.compile('(.*/external/[^/]+)/jar/(.*)[.]jar')
+    for p in self._query_classpath():
+      m = java_library.match(p)
       if m:
-        prefix = m.group(1)
-        suffix = m.group(2)
-        p = os.path.join(prefix, "src", "%s-src.jar" % suffix)
-        if os.path.exists(p):
-          s = p
-      classpathentry('lib', j, s)
+        src.add(m.group(1).lstrip('/'))
+      else:
+        if ext is not None and p.startswith("external"):
+          p = os.path.join(ext, p)
+          lib.add(p)
 
-  classpathentry('con', JRE)
-  classpathentry('output', 'eclipse-out/classes')
+    src_paths = {}
+    for s in sorted(src):
+      out = None
 
-  p = os.path.join(ROOT, '.classpath')
-  with open(p, 'w') as fd:
-    doc.writexml(fd, addindent='\t', newl='\n', encoding='UTF-8')
+      if s.startswith('lib/'):
+        out = 'eclipse-out/lib'
 
-def excluded(lib):
-  if args.exclude:
-    for x in args.exclude:
-      if x in lib:
-        return True
-  return False
+      p = os.path.join(s, 'java')
+      if os.path.exists(p):
+        classpathentry('src', p, out=out)
+        continue
 
-try:
-  name = args.name if args.name else os.path.basename(ROOT)
-  gen_project(name)
-  gen_classpath(retrieve_ext_location().decode('utf-8'))
+      for env in ['main', 'test', 'java', 'javatests']:
+        o = None
+        if out:
+          o = out + '/' + env
+        elif env == 'test' or env == 'javatests':
+          o = 'eclipse-out/test'
 
-except KeyboardInterrupt:
-  print('Interrupted by user', file=sys.stderr)
-  exit(1)
+        if s.startswith(env + '/'):
+          src_paths[env] = o
+          continue
+
+        for srctype in ['java', 'resources']:
+          p = os.path.join(s, 'src', env, srctype)
+          if os.path.exists(p):
+            src_paths[p] = o
+
+    for s in src_paths:
+      classpathentry('src', s, out=src_paths[s])
+
+    for libs in [lib]:
+      for j in sorted(libs):
+        if self.excluded(j):
+          continue
+        s = None
+        m = srcs.match(j)
+        if m:
+          prefix = m.group(1)
+          suffix = m.group(2)
+          p = os.path.join(prefix, "src", "%s-src.jar" % suffix)
+          if os.path.exists(p):
+            s = p
+        classpathentry('lib', j, s)
+
+    classpathentry('con', JRE)
+    classpathentry('output', 'eclipse-out/classes')
+
+    p = os.path.join(self.ROOT, '.classpath')
+    with open(p, 'w') as fd:
+      doc.writexml(fd, addindent='\t', newl='\n', encoding='UTF-8')
+
+  def excluded(self, lib):
+    if self.args.exclude:
+      for x in self.args.exclude:
+        if x in lib:
+          return True
+    return False
+
+def main():
+  try:
+    ec = EclipseProject()
+    ec.parse_args(args=sys.argv[1:])
+    ec.main()
+  except KeyboardInterrupt:
+    print('Interrupted by user', file=sys.stderr)
+    exit(1)
+
+
+if __name__ == '__main__':
+  main()
diff --git a/tools/eclipse/test_project.py b/tools/eclipse/test_project.py
new file mode 100755
index 0000000..be73474
--- /dev/null
+++ b/tools/eclipse/test_project.py
@@ -0,0 +1,132 @@
+#!/usr/bin/env python3
+#
+# You can run tests using:
+#
+#   bazel test --test_output=streamed //tools/eclipse:test
+#
+# Or directly:
+#
+# ./tools/eclipse/test_project.py
+#
+
+from io import StringIO
+import unittest
+from unittest.mock import call, patch
+from subprocess import CalledProcessError
+
+import project
+from project import EclipseProject
+
+class EclipseProjectTestCase(unittest.TestCase):
+
+  @patch.object(project, 'EclipseProject')
+  def test_script_entrypoint(self, ep):
+    with patch.object(project.sys, 'argv', ['project.py', '-r', '/dev/null']):
+      project.main()
+      ep.assert_has_calls([
+          call(),
+          call().parse_args(args=['-r', '/dev/null']),
+          call().main(),
+      ])
+
+  @patch('sys.stderr', new_callable=StringIO)
+  def test_script_entrypoint_handles_control_c(self, stderr):
+
+    with self.assertRaises(SystemExit) as c:
+      with patch.object(project.EclipseProject, 'parse_args',
+                        side_effect=KeyboardInterrupt):
+        project.main()
+
+    self.assertIn('Interrupted by user\n', stderr.getvalue())
+    self.assertEquals(c.exception.code, 1)
+
+  @patch('sys.stderr', new_callable=StringIO)
+  def test_requires_root_option(self, stderr):
+    with self.assertRaises(SystemExit) as c:
+      ep = EclipseProject()
+      ep.parse_args([])
+    self.assertEqual(c.exception.code, 2)
+    self.assertIn(
+      'the following arguments are required: -r/--root',
+      stderr.getvalue()
+    )
+
+  def test_batch_option_is_passed_to_bazel(self):
+      ep = EclipseProject()
+      ep.parse_args(['-r', '/dev/null'])
+      ep.bazel_exe = 'my_bazel'
+      self.assertEquals(ep._build_bazel_cmd(), ['my_bazel'])
+
+      ep.parse_args(['-r', '/dev/null', '--batch'])
+      self.assertEquals(ep._build_bazel_cmd(), ['my_bazel', '--batch'])
+
+  def test_find_root_raises_when_no_WORKSPACE_found(self):
+    with patch('os.path.exists') as exists:
+      exists.return_value = False
+      with self.assertRaises(Exception):
+        EclipseProject().find_root('/tmp/path/to')
+      exists.assert_has_calls([
+        call('/tmp/path/to/WORKSPACE'),
+        call('/tmp/path/WORKSPACE'),
+        call('/tmp/WORKSPACE'),
+      ])
+
+  def test_find_root_in_grandparent_directory(self):
+    with patch('os.path.exists') as exists:
+      exists.side_effect = [False, False, True, False]
+      root = EclipseProject().find_root('/tmp/path/to/foo')
+      exists.assert_has_calls([
+        call('/tmp/path/to/foo/WORKSPACE'),
+        call('/tmp/path/to/WORKSPACE'),
+        call('/tmp/path/WORKSPACE'),
+      ])
+      self.assertEqual('/tmp/path', root)
+
+  @patch('subprocess.check_output')
+  def test_find_bazel_finds_bazelisk_first(self, check_output):
+    check_output.return_value = b'/path/to/bazelisk'
+    self.assertEqual(EclipseProject().find_bazel(), '/path/to/bazelisk')
+
+  @patch('subprocess.check_output')
+  def test_find_bazel_fallback_to_bazel(self, check_output):
+    check_output.side_effect = [
+        CalledProcessError(1, 'which bazelisk',
+                           '', '[TEST] bazelisk not found'),
+        b'/path/to/bazel',
+    ]
+    self.assertEqual(EclipseProject().find_bazel(), '/path/to/bazel')
+
+  @patch('subprocess.check_output')
+  def test_find_bazel_raise_without_bazel_and_bazelisk(self, check_output):
+    check_output.side_effect = [
+        CalledProcessError(1, 'which bazelisk',
+                           '', '[TEST] bazelisk not found'),
+        CalledProcessError(1, 'which bazel',
+                           '', '[TEST] bazel not found'),
+    ]
+    with self.assertRaisesRegex(Exception, "Neither bazelisk nor bazel found"):
+      EclipseProject().find_bazel()
+
+  @patch('subprocess.check_output')
+  def test_find_given_existing_bazel_exe(self, check_output):
+    check_output.return_value = b'/path/to/bin/mybazel'
+    self.assertEqual(EclipseProject().find_bazel('my_bazel'),
+                     '/path/to/bin/mybazel')
+
+  @patch('subprocess.check_output')
+  def test_find_given_not_existing_bazel_exe(self, check_output):
+    check_output.side_effect = CalledProcessError(
+      1, 'which mybazel', '', '[TEST] mybazel not found')
+    with self.assertRaisesRegex(Exception, 'Bazel command: mybazel not found'):
+      EclipseProject().find_bazel('mybazel')
+
+  @patch('subprocess.check_output')
+  def test_retrieve_ext_location_strips_newline(self, check_output):
+    check_output.return_value = '/path/to/.cache/bazel/xxxx\n'
+    ep = EclipseProject()
+    ep.parse_args(['-r' '/dev/null', '--bazel', 'my_bazel'])
+    assert not ep.retrieve_ext_location().endswith('\n')
+
+
+if __name__ == "__main__":
+  unittest.main(verbosity=2)
diff --git a/tools/eclipse/test_project_coverage b/tools/eclipse/test_project_coverage
new file mode 100755
index 0000000..fa3e819
--- /dev/null
+++ b/tools/eclipse/test_project_coverage
@@ -0,0 +1,6 @@
+#!/bin/bash
+
+COVERAGE="python3 -m coverage"
+
+$COVERAGE run --branch --include=project.py test_project.py
+$COVERAGE html