From 3226fa8aa211fb5f72c535c03b24ae91838cc2f6 Mon Sep 17 00:00:00 2001
From: Don McCasland <donmccasland@google.com>
Date: Tue, 12 Feb 2019 15:17:46 -0800
Subject: [PATCH] Addressing review comments

---
 hack/prow/run_tests.py | 61 ++++++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 23 deletions(-)

diff --git a/hack/prow/run_tests.py b/hack/prow/run_tests.py
index c985a79667..c339cf4f3c 100755
--- a/hack/prow/run_tests.py
+++ b/hack/prow/run_tests.py
@@ -39,10 +39,15 @@ def write_results(outdir, started, finished, test_results):
                       "result":SUCCESS|FAIL,
                       "metadata":{}
                       }
+     Args:
+       outdir: a string containing the results storage directory
+       started: a dict containing the starting data
+       finished: a dict containing the finished data
+       tests_results: a list of dicts containing test results
   """
-  started_json_file = open('%s/started.json' % outdir, 'w')
-  finished_json_file = open('%s/finished.json' % outdir, 'w')
-  junit_xml_file = open('%s/artifacts/junit_runner.xml' % outdir, 'w')
+  started_json = open(os.path.join(outdir, "started.json"), 'w')
+  finished_json = open(os.path.join(outdir, "finished.json"), 'w')
+  junit_xml = open(os.path.join(outdir, "artifacts", "junit_runner.xml"), 'w')
 
   failures = 0
   testxml = ""
@@ -52,30 +57,33 @@ def write_results(outdir, started, finished, test_results):
         failures += 1
         testxml += '<failure message="Test Failed" />'
     testxml += '</testcase>\n'
-  junit_xml_file.write('<testsuite failures="%s" tests="%s">\n' % (failures, len(test_results)))
-  junit_xml_file.write(testxml)
-  junit_xml_file.write('</testsuite>')
-  junit_xml_file.close()
+  junit_xml.write('<testsuite failures="%s" tests="%s">\n' % (failures, len(test_results)))
+  junit_xml.write(testxml)
+  junit_xml.write('</testsuite>')
+  junit_xml.close()
 
-  started_json_file.write(json.dumps(started)) 
-  started_json_file.close()
-  finished_json_file.write(json.dumps(finished)) 
-  finished_json_file.close()
+  started_json.write(json.dumps(started)) 
+  started_json.close()
+  finished_json.write(json.dumps(finished)) 
+  finished_json.close()
 
   return
 
 def upload_results(outdir, test, buildnum, bucket):
-  """ push the contents of gcs_out/* into bucket/test/logs/buildnum"""
+  """ push the contents of gcs_out/* into bucket/test/logs/buildnum
+
+      Args:
+       outdir: a string containing the results storage directory
+       test: a string containing path to the test script
+       buildnum: a string containing the buildnum
+       bucket: a string containing the bucket to upload results to
+  """
   classname = os.path.basename(test).split('.')[0]
   args = shlex.split("gsutil cp -R gcs_out/ gs://%s/logs/%s/%s" % (bucket, classname, buildnum))
   p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
   for line in p.stdout:
     print line
 
-def gather_artifacts():
-  """ gather any named or default artifacts into artifacts/ """
-  return
-
 def run_tests(test, build_log, exit_status, started, finished, test_results):
   """ execute the test, grab the start time, finish time, build logs and exit status
       Pull test results and important information out of the build log 
@@ -84,6 +92,14 @@ def run_tests(test, build_log, exit_status, started, finished, test_results):
       --- PASS: TestFunctional (42.87s)
           --- PASS: TestFunctional/Status (2.33s)
           --- FAIL: SOMETESTSUITE/TESTNAME (seconds)
+
+      Args:
+       test: a string containing path to the test script
+       build_log: a string containing path to the build_log
+       exit_status: a string that will contain the test script's exit_status
+       started: a dict containing the starting data
+       finished: a dict containing the finished data
+       tests_results: a list of dicts containing test results
   """
   classname = os.path.basename(test).split('.')[0]
   build_log_file = open(build_log, 'w')
@@ -110,18 +126,17 @@ def main(argv):
   parser.add_argument('--buildnum', required=True, help='buildnumber for uploading to GCS')
   parser.add_argument('--bucket', default="k8s-minikube-prow", help='Name of the GCS bucket to upload to.  Default: k8s-minkube-prow')
   parser.add_argument('--outdir', default="gcs_out", help='Path of the directory to store all results, artifacts, and logs')
-  parser.add_argument('--artifact', help='SRCPATH:TARGETPATH use to specify a file that needs to be uploaded into GCS bucket')
   args = parser.parse_args()
 
   if not os.path.exists(args.outdir):
     os.makedirs(args.outdir)
-    os.makedirs('%s/artifacts' % args.outdir)
+    os.makedirs(os.path.join(args.outdir, "artifacts"))
 
-  build_log="%s/build_log.txt" % (args.outdir)
-  exit_status=""
-  started={"timestamp":calendar.timegm(time.gmtime())}
-  finished={}
-  test_results=[]
+  build_log = os.path.join(args.outdir, "build_log.txt")
+  exit_status = ""
+  started = {"timestamp":calendar.timegm(time.gmtime())}
+  finished = {}
+  test_results = []
 
   run_tests(args.test, build_log, exit_status, started, finished, test_results)