Store files locally in a different location after they've been posted.

tools/testing/perf_testing/run_perf_tests.py

 #!/usr/bin/python
 
 # Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 # for details. All rights reserved. Use of this source code is governed by a
 # BSD-style license that can be found in the LICENSE file.
 
 import datetime
 import math             
 try:            
   from matplotlib.font_manager import FontProperties            
@@ skipping 126 matching lines @@
           print 'Broken Build'
           return 1
     return 0
 
   def ensure_output_directory(self, dir_name):
     """Test that the listed directory name exists, and if not, create one for
     our output to be placed.
 
     Args:
       dir_name: the directory we will create if it does not exist."""
-    dir_path = os.path.join(DART_INSTALL_LOCATION, 'tools', 
-                            'testing', 'perf_testing', dir_name)
-    if not os.path.exists(dir_path):
-      os.mkdir(dir_path)
-      print 'Creating output directory ', dir_path
+    for directory in [dir_name, 'old', os.path.join('old', dir_name)]:
+      dir_path = os.path.join(DART_INSTALL_LOCATION, 'tools', 
+                              'testing', 'perf_testing', directory)
+      if not os.path.exists(dir_path):
+        os.mkdir(dir_path)

[vsm, 2012/05/07 20:25:54]

You can eliminate 'old' from your list and just call makedirs (recursive dir
create) here.  Consider unrolling the loop.  I think a local function is a
cleaner way to factor out redundancy.  E.g.,

make_if_necessary(dir_name)
make_if_necessary(os.path.join('old', dir_name)

[Emily Fortuna, 2012/05/08 01:24:51]

Done.

+        print 'Creating output directory ', dir_path
 
   def has_new_code(self):
     """Tests if there are any newer versions of files on the server."""
     os.chdir(DART_INSTALL_LOCATION)
     # Pass 'p' in if we have a new certificate for the svn server, we want to
     # (p)ermanently accept it.
-    results = self.run_cmd(['svn', 'st', '-u'], std_in='p')
+    results = self.run_cmd(['svn', 'st', '-u'], std_in='p\r\n')
     for line in results:
       if '*' in line:
         return True
     return False
     
   def get_os_directory(self):
     """Specifies the name of the directory for the testing build of dart, which
     has yet a different naming convention from utils.getBuildRoot(...)."""
     if platform.system() == 'Windows':
       return 'windows'
@@ skipping 175 matching lines @@
     for visitor in [self.tester, self.file_processor, self.grapher]:
       visitor.prepare()
     
     os.chdir(DART_INSTALL_LOCATION)
     self.test_runner.ensure_output_directory(self.result_folder_name)
     if not self.test_runner.no_test:
       self.tester.run_tests()
 
     os.chdir(os.path.join('tools', 'testing', 'perf_testing'))
 
-    files = os.listdir(self.result_folder_name)
-    for afile in files:
-      if not afile.startswith('.'):
-        self.file_processor.process_file(afile)
-
-    if 'plt' in globals():
-      # Only run Matplotlib if it is installed.
-      self.grapher.plot_results('%s.png' % self.result_folder_name)
+    for directory in [os.path.join('old', self.result_folder_name),
+                      self.result_folder_name]:

[vsm, 2012/05/07 20:25:54]

I would unroll this loop for clarity also - especially given the if old_dir / if
not old_dir in the body.

[Emily Fortuna, 2012/05/08 01:24:51]

Done.

+      old_dir = '/old/' in directory or '\\old\\' in directory
+      files = os.listdir(directory)
+      for afile in files:
+        if not afile.startswith('.'):
+          if self.file_processor.process_file(afile, not old_dir) and \

[vsm, 2012/05/07 20:25:54]

I'd take process_file out of the conditional.  It's easy to miss side-effecting
code in there.

[Emily Fortuna, 2012/05/08 01:24:51]

Done.

+              not old_dir:
+            shutil.move(os.path.join(self.result_folder_name, afile),
+                        os.path.join('old', self.result_folder_name, afile))
+      if 'plt' in globals() and old_dir:
+        # Only run Matplotlib if it is installed.
+        self.grapher.plot_results('%s.png' % self.result_folder_name)
 
 
 class Tester(object):
   """The base level visitor class that runs tests. It contains convenience 
   methods that many Tester objects use. Any class that would like to be a
   TesterVisitor must implement the run_tests() method."""
 
   def __init__(self, test):
     self.test = test
 
@@ skipping 39 matching lines @@
   COMPILE_TIME = 'CompileTime'
   CODE_SIZE = 'CodeSize'
 
   def __init__(self, test):
     self.test = test
 
   def prepare(self):
     """Perform any initial setup required before the test is run."""
     pass
 
-  def should_report_results(self, afile):
-    """We store all trace files locally, but we don't want to post all of the
-    results every time, so we only attempt to post results for recent runs."""
-    cur_time = time.time()
-    file_mod_time = os.path.getmtime(os.path.join(
-        self.test.result_folder_name, afile))
-    return cur_time - file_mod_time < 1000 # Files modified in the last ~15 min.
-
   def report_results(self, benchmark_name, score, platform, variant, 
                      revision_number, metric):
     """Store the results of the benchmark run.
     Args:
       benchmark_name: The name of the individual benchmark.
       score: The numerical value of this benchmark.
       platform: The platform the test was run on (firefox, command line, etc).
       variant: Specifies whether the data was about generated Frog, js, a
           combination of both, or Dart depending on the test.
       revision_number: The revision of the code (and sometimes the revision of
@@ skipping 235 matching lines @@
           file_path = os.path.join(
               os.getcwd(), 'internal', 'browserBenchmarks',
               'benchmark_page_%s.html' % version)
           self.test.test_runner.run_cmd(
               ['python', os.path.join('tools', 'testing', 'run_selenium.py'),
               '--out', file_path, '--browser', browser,
               '--timeout', '600', '--mode', 'perf'], self.test.trace_file, 
               append=True)
 
   class CommonBrowserFileProcessor(Processor):
-    def process_file(self, afile):
+    def process_file(self, afile, should_post_file):
       """Comb through the html to find the performance results.
       Returns: True if we successfully posted our data to storage and/or we can
           delete the trace file."""
       os.chdir(os.path.join(DART_INSTALL_LOCATION, 'tools',
                             'testing', 'perf_testing'))
       parts = afile.split('-')
       browser = parts[2]
       version = parts[3]
       f = open(os.path.join(self.test.result_folder_name, afile))
       lines = f.readlines()
@@ skipping 24 matching lines @@
         name_and_score = result.split(':')
         if len(name_and_score) < 2:
           break
         name = name_and_score[0].strip()
         score = name_and_score[1].strip()
         if version == 'js' or version == 'v8':
           version = 'js'
         bench_dict = self.test.values_dict[browser][version]
         bench_dict[name] += [float(score)]
         self.test.revision_dict[browser][version][name] += [revision_num]
-        if self.should_report_results(afile) and \
-            not self.test.test_runner.no_upload:
+        if not self.test.test_runner.no_upload and should_post_file:
           upload_success = upload_success and self.report_results(
               name, score, browser, version, revision_num, self.SCORE)
         else:
           upload_success = False
 
       f.close()
       self.calculate_geometric_mean(browser, version, revision_num)
       return upload_success
 
 
@@ skipping 122 matching lines @@
     @staticmethod
     def get_dromaeo_url_query(browser, version):
       if browser == 'dartium':
         version = version.replace('frog', 'dart')
       version = version.replace('_','&')
       tags = DromaeoTester.get_valid_dromaeo_tags()
       return '|'.join([ '%s&%s' % (version, tag) for tag in tags])
 
 
   class DromaeoFileProcessor(Processor):
-    def process_file(self, afile):
+    def process_file(self, afile, should_post_file):
       """Comb through the html to find the performance results.
       Returns: True if we successfully posted our data to storage."""
       parts = afile.split('-')
       browser = parts[2]
       version = parts[3]
 
       bench_dict = self.test.values_dict[browser][version]
 
       f = open(os.path.join(self.test.result_folder_name, afile))
       lines = f.readlines()
@@ skipping 15 matching lines @@
           for suite_result in suite_results:
             results = re.findall(r'<li>(.*?)</li>', suite_result)
             if results:
               for result in results:
                 r = re.match(result_pattern, result)
                 name = DromaeoTester.legalize_filename(r.group(1).strip(':'))
                 score = float(r.group(2))
                 bench_dict[name] += [float(score)]
                 self.test.revision_dict[browser][version][name] += \
                     [revision_num]
-                if self.should_report_results(afile) and \
-                    not self.test.test_runner.no_upload:
+                if not self.test.test_runner.no_upload and should_post_file:
                   upload_success = upload_success and self.report_results(
                       name, score, browser, version, revision_num, self.SCORE)
                 else:
                   upload_success = False
 
       f.close()
       self.calculate_geometric_mean(browser, version, revision_num)
       return upload_success
 
 
@@ skipping 72 matching lines @@
                                             self.test.extra_metrics[0])],
           self.test.trace_file, append=True)
       for (variant, _) in variants:
         self.test.test_runner.run_cmd(
             ['echo', 'Size (%s, %s): %s' % (variant, self.test.extra_metrics[0],
                                             total_size[variant])],
             self.test.trace_file, append=True)
 
 
   class DromaeoSizeProcessor(Processor):
-    def process_file(self, afile):
+    def process_file(self, afile, should_post_file):
       """Pull all the relevant information out of a given tracefile.
 
       Args:
         afile: is the filename string we will be processing.
       Returns: True if we successfully posted our data to storage."""
       os.chdir(os.path.join(DART_INSTALL_LOCATION, 'tools',
           'testing', 'perf_testing'))
       f = open(os.path.join(self.test.result_folder_name, afile))
       tabulate_data = False
       revision_num = 0
@@ skipping 12 matching lines @@
           variant = result.group(1)
           metric = result.group(2)
           num = result.group(3)
           if num.find('.') == -1:
             num = int(num)
           else:
             num = float(num)
           self.test.values_dict['commandline'][variant][metric] += [num]
           self.test.revision_dict['commandline'][variant][metric] += \
               [revision_num]
-          if self.should_report_results(afile) and \
-              not self.test.test_runner.no_upload:
+          if not self.test.test_runner.no_upload and should_post_file:
             upload_success = upload_success and self.report_results(
                 metric, num, 'commandline', variant, revision_num,
                 self.CODE_SIZE)
           else:
             upload_success = False
 
       f.close()
       return upload_success
 
   class DromaeoSizeGrapher(Grapher):
@@ skipping 75 matching lines @@
 
       self.test.test_runner.run_cmd(
           ['echo', '%d Generated checked total size' % total_size],
           self.test.trace_file, append=True)
     
       os.chdir('..')
 
 
   class CompileProcessor(Processor):
 
-    def process_file(self, afile):
+    def process_file(self, afile, should_post_file):
       """Pull all the relevant information out of a given tracefile.
 
       Args:
         afile: is the filename string we will be processing.
       Returns: True if we successfully posted our data to storage."""
       os.chdir(os.path.join(DART_INSTALL_LOCATION, 'tools',
           'testing', 'perf_testing'))
       f = open(os.path.join(self.test.result_folder_name, afile))
       tabulate_data = False
       revision_num = 0
       upload_success = True
       for line in f.readlines():
         tokens = line.split()
         if 'Revision' in line:
           revision_num = int(line.split()[1])
         else:
           for metric in self.test.values_list:
             if metric in line:
               num = tokens[0]
               if num.find('.') == -1:
                 num = int(num)
               else:
                 num = float(num)
               self.test.values_dict['commandline']['frog'][metric] += [num]
               self.test.revision_dict['commandline']['frog'][metric] += \
                   [revision_num]
               score_type = self.CODE_SIZE
               if 'Compiling' in metric or 'Bootstrapping' in metric:
                 score_type = self.COMPILE_TIME
-              if self.should_report_results(afile) and \
-                  not self.test.test_runner.no_upload:
+              if not self.test.test_runner.no_upload and should_post_file:
                 if num < self.test.failure_threshold[metric]:
                   num = 0
                 upload_success = upload_success and self.report_results(
                     metric, num, 'commandline', 'frog', revision_num,
                     score_type)
               else:
                 upload_success = False
       if revision_num != 0:
         for metric in self.test.values_list:
           self.test.revision_dict['commandline']['frog'][metric].pop()
@@ skipping 38 matching lines @@
     while True:
       if runner.has_new_code():
         runner.run_test_sequence()
       else:
         time.sleep(200)
   else:
     runner.run_test_sequence()
 
 if __name__ == '__main__':
   main()