Revert test_py_lint.py (kubeflow#496)

* Other workflows are using test_py_lint and making it a pytest breaks them.

* Create a new version py_lint_test.py for people that want to migrate.

py/kubeflow/testing/ci/kf_unittests.py

@@ -215,7 +215,7 @@ def build(self):
 
     py_lint["name"] = "py-lint"
     py_lint["container"]["command"] = ["pytest",
-                                       "test_py_lint.py",
+                                       "py_lint_test.py",
                                        # I think -s mean stdout/stderr will
                                        # print out to aid in debugging.
                                        # Failures still appear to be captured

py/kubeflow/testing/py_lint_test.py

@@ -0,0 +1,90 @@
+import fnmatch
+import logging
+import os
+import subprocess
+
+from kubeflow.testing import util
+
+import pytest
+
+logging.basicConfig(
+    level=logging.INFO,
+    format=('%(levelname)s|%(asctime)s'
+            '|%(pathname)s|%(lineno)d| %(message)s'),
+    datefmt='%Y-%m-%dT%H:%M:%S',
+)
+logging.getLogger().setLevel(logging.INFO)
+
+def should_exclude(root, full_dir_excludes):
+  for e in full_dir_excludes:
+    if root.startswith(e):
+      return True
+  return False
+
+
+def test_lint(record_xml_attribute, src_dir, rcfile): # pylint: disable=redefined-outer-name
+  # Override the classname attribute in the junit file.
+  # This makes it easy to group related tests in test grid.
+  # http://doc.pytest.org/en/latest/usage.html#record-xml-attribute
+  util.set_pytest_junit(record_xml_attribute, "test_py_lint")
+
+  logging.info('Running test_lint')
+  # Print out the pylint version because different versions can produce
+  # different results.
+  util.run(["pylint", "--version"])
+
+  # kubeflow_testing is imported as a submodule so we should exclude it
+  # TODO(jlewi): We should make this an argument.
+  dir_excludes = [
+    "dashboard/frontend/node_modules",
+    "kubeflow_testing",
+    "dev-kubeflow-org/ks-app/vendor",
+    "release-infra",
+  ]
+  full_dir_excludes = [
+    os.path.join(os.path.abspath(src_dir), f) for f in dir_excludes
+  ]
+
+  # TODO(jlewi): Use pathlib once we switch to python3.
+  includes = ["*.py"]
+  failed_files = []
+  if not rcfile:
+    rcfile = os.path.join(src_dir, ".pylintrc")
+
+  for root, dirs, files in os.walk(os.path.abspath(src_dir), topdown=True):
+    # Exclude vendor directories and all sub files.
+    if "vendor" in root.split(os.sep):
+      continue
+
+    # excludes can be done with fnmatch.filter and complementary set,
+    # but it's more annoying to read.
+    if should_exclude(root, full_dir_excludes):
+      continue
+
+    dirs[:] = [d for d in dirs]
+    for pat in includes:
+      for f in fnmatch.filter(files, pat):
+        full_path = os.path.join(root, f)
+        try:
+          util.run(
+            ["pylint", "--rcfile=" + rcfile, full_path], cwd=src_dir)
+        except subprocess.CalledProcessError:
+          failed_files.append(full_path[len(src_dir):])
+  if failed_files:
+    failed_files.sort()
+    logging.error("%s files had lint errors:\n%s", len(failed_files),
+                  "\n".join(failed_files))
+  else:
+    logging.info("No lint issues.")
+
+  assert not failed_files
+
+if __name__ == "__main__":
+  logging.basicConfig(
+      level=logging.INFO,
+      format=('%(levelname)s|%(asctime)s'
+              '|%(pathname)s|%(lineno)d| %(message)s'),
+      datefmt='%Y-%m-%dT%H:%M:%S',
+  )
+  logging.getLogger().setLevel(logging.INFO)
+  pytest.main()

py/kubeflow/testing/test_py_lint.py

@@ -1,19 +1,12 @@
+"""TODO(jlewi): This is deprecated. New code should use py_lint_test.py"""
+import argparse
 import fnmatch
 import logging
 import os
 import subprocess
 
-from kubeflow.testing import util
+from kubeflow.testing import test_helper, util
 
-import pytest
-
-logging.basicConfig(
-    level=logging.INFO,
-    format=('%(levelname)s|%(asctime)s'
-            '|%(pathname)s|%(lineno)d| %(message)s'),
-    datefmt='%Y-%m-%dT%H:%M:%S',
-)
-logging.getLogger().setLevel(logging.INFO)
 
 def should_exclude(root, full_dir_excludes):
   for e in full_dir_excludes:
@@ -22,13 +15,28 @@ def should_exclude(root, full_dir_excludes):
   return False
 
 
-def test_lint(record_xml_attribute, src_dir, rcfile): # pylint: disable=redefined-outer-name
-  # Override the classname attribute in the junit file.
-  # This makes it easy to group related tests in test grid.
-  # http://doc.pytest.org/en/latest/usage.html#record-xml-attribute
-  util.set_pytest_junit(record_xml_attribute, "test_py_lint")
+def parse_args():
+  parser = argparse.ArgumentParser()
+  parser.add_argument(
+    "--src_dir",
+    default=os.getcwd(),
+    type=str,
+    help=("The root directory of the source tree. Defaults to current "
+          "directory."))
+
+  parser.add_argument(
+    "--rcfile",
+    default="",
+    type=str,
+    help=("Path to the rcfile."))
+  args, _ = parser.parse_known_args()
+  return args
 
-  logging.info('Running test_lint')
+
+def test_lint(test_case): # pylint: disable=redefined-outer-name
+  logging.warning('test_py_lint.py is deprecated in favor of '
+                  'py_lint_test.py which uses pytest')
+  args = parse_args()
   # Print out the pylint version because different versions can produce
   # different results.
   util.run(["pylint", "--version"])
@@ -42,16 +50,18 @@ def test_lint(record_xml_attribute, src_dir, rcfile): # pylint: disable=redefine
     "release-infra",
   ]
   full_dir_excludes = [
-    os.path.join(os.path.abspath(src_dir), f) for f in dir_excludes
+    os.path.join(os.path.abspath(args.src_dir), f) for f in dir_excludes
   ]
 
   # TODO(jlewi): Use pathlib once we switch to python3.
   includes = ["*.py"]
   failed_files = []
-  if not rcfile:
-    rcfile = os.path.join(src_dir, ".pylintrc")
+  if not args.rcfile:
+    rc_file = os.path.join(args.src_dir, ".pylintrc")
+  else:
+    rc_file = args.rcfile
 
-  for root, dirs, files in os.walk(os.path.abspath(src_dir), topdown=True):
+  for root, dirs, files in os.walk(os.path.abspath(args.src_dir), topdown=True):
     # Exclude vendor directories and all sub files.
     if "vendor" in root.split(os.sep):
       continue
@@ -67,24 +77,20 @@ def test_lint(record_xml_attribute, src_dir, rcfile): # pylint: disable=redefine
         full_path = os.path.join(root, f)
         try:
           util.run(
-            ["pylint", "--rcfile=" + rcfile, full_path], cwd=src_dir)
+            ["pylint", "--rcfile=" + rc_file, full_path], cwd=args.src_dir)
         except subprocess.CalledProcessError:
-          failed_files.append(full_path[len(src_dir):])
+          failed_files.append(full_path[len(args.src_dir):])
   if failed_files:
     failed_files.sort()
+    test_case.add_failure_info("Files with lint issues: {0}".format(
+      ", ".join(failed_files)))
     logging.error("%s files had lint errors:\n%s", len(failed_files),
                   "\n".join(failed_files))
   else:
     logging.info("No lint issues.")
 
-  assert not failed_files
 
 if __name__ == "__main__":
-  logging.basicConfig(
-      level=logging.INFO,
-      format=('%(levelname)s|%(asctime)s'
-              '|%(pathname)s|%(lineno)d| %(message)s'),
-      datefmt='%Y-%m-%dT%H:%M:%S',
-  )
-  logging.getLogger().setLevel(logging.INFO)
-  pytest.main()
+  test_case = test_helper.TestCase(name='test_lint', test_func=test_lint)
+  test_suite = test_helper.init(name='py_lint', test_cases=[test_case])
+  test_suite.run()