Merge pull request #3002 from cgolubi1/2953_replay_test_improvements

Implement some improvements to random_ai and replay_loop

tools/api-client/python/lib/random_ai.py

@@ -330,11 +330,9 @@ def _write_entry_type_reactToReserve(self, entry):
   def _write_entry_type_initialGameData(self, entry):
     data = self.apply_known_changes_to_game_data(entry['data'])
     self.f.write("""
-        $expData = $this->squash_game_data_timestamps(%s);
-        $expData['gameId'] = $gameId;
-        $expData['playerDataArray'][0]['playerId'] = $this->user_ids['responder003'];
-        $expData['playerDataArray'][1]['playerId'] = $this->user_ids['responder004'];
-""" % self._php_json_decode_blob(data))
+        $expData = $this->generate_init_expected_data_array($gameId, 'responder003', 'responder004', %(maxWins)s, '%(gameState)s');
+""" % data)
+    self._write_php_json_diff(data, {})
     self.olddata = data
 
   def _write_entry_type_updatedGameData(self, entry):
@@ -482,10 +480,11 @@ def _write_php_diff_string_key(self, suffix, newval, oldval):
       self.f.write("        $expData%s = %s;\n" % (suffix, jsonstr))
 
   def _write_php_diff_action_log(self, keyname, newval, oldval):
-    # Don't think this can actually happen, but handle it just in case
-    if len(newval) == 0:
+    # If we're starting from scratch, populate the object
+    if not oldval or not newval:
       self.f.write("        $expData['gameActionLog'] = array();\n");
-      return
+      # This probably can't happen, but just in case
+      if not newval: return
     nextkey = len(newval) - 1
     # If newval is large enough to imply that we are at the end of
     # the game, empty it and start over
@@ -647,19 +646,22 @@ def _write_php_diff_player_data_array(self, key, pnum, newdata, olddata):
         if pkey in olddata: oldval = olddata[pkey]
         self._write_php_diff_player_data_die_array(pnum, pkey, newdata[pkey], oldval)
       elif pkey in ['gameScoreArray', 'swingRequestArray', 'prevSwingValueArray', ]:
-        self._write_php_diff_flat_dict_key(suffix, newdata[pkey], olddata[pkey])
+        self._write_php_diff_flat_dict_key(suffix, newdata[pkey], olddata.get(pkey, {}))
       elif pkey in ['button', ]:
-        self._write_php_diff_flat_dict_key(suffix, newdata[pkey], olddata[pkey], True)
+        self._write_php_diff_flat_dict_key(suffix, newdata[pkey], olddata.get(pkey, {}), True)
       elif pkey in ['prevOptValueArray', ]:
-        self._write_php_diff_prev_opt_value_array(suffix, newdata[pkey], olddata[pkey])
+        self._write_php_diff_prev_opt_value_array(suffix, newdata[pkey], olddata.get(pkey, {}))
       elif pkey in ['optRequestArray', 'turboSizeArray', ]:
-        self._write_php_diff_opt_request_array(suffix, newdata[pkey], olddata[pkey])
+        self._write_php_diff_opt_request_array(suffix, newdata[pkey], olddata.get(pkey, {}))
       elif pkey in NUMERIC_KEYS:
-        self._write_php_diff_numeric_key(suffix, newdata[pkey], olddata[pkey])
+        self._write_php_diff_numeric_key(suffix, newdata[pkey], olddata.get(pkey, None))
       elif pkey in UNUSED_DURING_AUTOPLAY_KEYS:
-        if newdata[pkey] != olddata[pkey]:
+        if olddata and newdata[pkey] != olddata[pkey]:
           self.bug("Playerdata key %s is expected to be static, but unexpectedly changed between loadGameData invocations: %s => %s" % (
             pkey, olddata[pkey], newdata[pkey]))
+	# If olddata isn't defined because we're in game initialization, don't fail.
+        # Don't do anything else either, because nothing else should be needed:
+        # $this->generate_init_expected_data_array() should initialize these items.
       elif pkey == 'lastActionTime':
         pass
       else:
@@ -669,24 +671,28 @@ def _write_php_json_diff(self, newobj, oldobj):
     for key in sorted(newobj.keys()):
       suffix = "['%s']" % key
       if key in NUMERIC_KEYS:
-        self._write_php_diff_numeric_key(suffix, newobj[key], oldobj[key])
+        self._write_php_diff_numeric_key(suffix, newobj[key], oldobj.get(key, None))
       elif key in STRING_KEYS:
-        self._write_php_diff_string_key(suffix, newobj[key], oldobj[key])
+        self._write_php_diff_string_key(suffix, newobj[key], oldobj.get(key, None))
       elif key == 'gameActionLog':
-        self._write_php_diff_action_log(key, newobj[key], oldobj[key])
+        self._write_php_diff_action_log(key, newobj[key], oldobj.get(key, []))
       elif key in UNUSED_DURING_AUTOPLAY_KEYS:
-        if newobj[key] != oldobj[key]:
+        if oldobj and newobj[key] != oldobj[key]:
           self.bug("Key %s is expected to be static, but unexpectedly changed between loadGameData invocations: %s => %s" % (
             key, oldobj[key], newobj[key]))
+	# If oldobj isn't defined because we're in game initialization, don't fail.
+        # Don't do anything else either, because nothing else should be needed:
+        # $this->generate_init_expected_data_array() should initialize these items.
       elif key == 'playerDataArray':
         for pnum in range(len(newobj[key])):
-          self._write_php_diff_player_data_array(key, pnum, newobj[key][pnum], oldobj[key][pnum])
+          old_player_data = oldobj[key][pnum] if oldobj else {}
+          self._write_php_diff_player_data_array(key, pnum, newobj[key][pnum], old_player_data)
       elif key == 'timestamp':
         pass
       elif key == 'validAttackTypeArray':
-        self._write_php_diff_flat_array_key(suffix, newobj[key], oldobj[key])
+        self._write_php_diff_flat_array_key(suffix, newobj[key], oldobj.get(key, []))
       elif key == 'gameSkillsInfo':
-        self._write_php_diff_game_skills_info(suffix, newobj[key], oldobj[key])
+        self._write_php_diff_game_skills_info(suffix, newobj[key], oldobj.get(key, None))
       else:
         raise ValueError, key
 

tools/api-client/python/replaytest/replay_loop

@@ -81,6 +81,9 @@ between the actions it has been configured to take:
   parser.add_argument(
     '--local-replay', '-l', action='store_true',
     help="replay each batch of novel games locally after recording it")
+  parser.add_argument(
+    '--test-output', '-o', action='store_true',
+    help="replay the games currently recorded in the output directory")
   parser.add_argument(
     '--skip-init', '-s', action='store_true',
     help="skip initial batch of novel games, so the first action is replaying a batch")
@@ -167,7 +170,7 @@ def generate_responder_testfile(gen_command, output_file):
     'echo "<?php" | %s tee -a %s' % (write_to_bm_prefix, output_file),
     'echo "require_once \'responderTestFramework.php\';" | %s tee -a %s' % (write_to_bm_prefix, output_file),
     'echo "class responder99Test extends responderTestFramework {" | %s tee -a %s' % (write_to_bm_prefix, output_file),
-    '%s ./output | %s tee -a %s > /dev/null' % (gen_command, write_to_bm_prefix, output_file),
+    'bash -o pipefail -c "%s ./output | %s tee -a %s > /dev/null"' % (gen_command, write_to_bm_prefix, output_file),
     'echo "}" | %s tee -a %s > /dev/null' % (write_to_bm_prefix, output_file),
   ]
   if os.path.isfile(output_file):
@@ -234,49 +237,64 @@ def player_two_button_args():
     return 'name=%s' % ARGS.opponent_button_names
   return ''
 
-def test_new_games():
-  # If we're running in archive mode, this will generate new games
-  # and archive them for replay testing on this and other sites.
-  # Otherwise, it will simply run the tests and discard the results.
-  #
-  # Regardless, this is intended to blow up if any exceptions are
-  # received.
-
-  # Restart MySQL and apache, then reset primary and test databases
-  restart_mysqld()
-  restart_apache()
+def recreate_buttonmen_databases():
   os.system('echo "drop database buttonmen" | sudo mysql')
   os.system('sudo /usr/local/bin/create_buttonmen_databases')
   os.system('cat ~/example_players.sql | sudo mysql')
 
+# Actually play novel games if that's correct for the arguments we're using
+# Fail if any exceptions are received
+def generate_new_games(timestamp):
+  if ARGS.test_output: return
+
   os.chdir(HOMEBMDIR)
+
+  # Restart MySQL and apache, then reset primary and test databases
+  restart_mysqld()
+  restart_apache()
+  recreate_buttonmen_databases()
+
+  # This command runs the games
   cmdargs = './test_log_games %d "%s" "%s"' % (
     ARGS.num_games, player_one_button_args(), player_two_button_args())
   retval = os.system(cmdargs)
   if retval != 0:
     sys.exit(1)
 
+  # Capture the database signature of the new games for sanity-checking
+  # and easy stats about what's been tested
+  log_database_games('new_games.%s' % timestamp)
+
+  # Always immediately syntax-test the output files we just generated
+  # In archive mode, this is what creates ./output/allgames.php
   target_file = ARGS.archive_games and './output/allgames.php' or '/dev/null'
   retval = os.system('./prep_replay_games ./output > %s' % target_file)
   if retval != 0:
     sys.exit(1)
 
-  timestamp = datetime.datetime.now().strftime('%Y%m%d.%H%M%S')
-  log_database_games('new_games.%s' % timestamp)
+def replay_generated_games(timestamp):
+  if not (ARGS.archive_games or ARGS.local_replay or ARGS.test_output): return
 
-  if ARGS.local_replay:
-    generate_responder_testfile('./prep_replay_games', TESTFILE)
-    testname = 'local.%s' % timestamp
-    execute_responder_test(testname)
-    if not phpunit_log_shows_success(testname):
-      sys.exit(1)
+  os.chdir(HOMEBMDIR)
 
-  if ARGS.archive_games:
-    targetpath = '%s/%s.games.%s.tar' % (GAMESDIR, COMMITID, timestamp)
-    os.system('tar cf %s ./output' % (targetpath))
-    os.system('bzip2 %s' % (targetpath))
+  generate_responder_testfile('./prep_replay_games', TESTFILE)
+  testname = 'local.%s' % timestamp
+  execute_responder_test(testname)
+  if not phpunit_log_shows_success(testname):
+    sys.exit(1)
+
+def archive_generated_games(timestamp):
+  if not ARGS.archive_games: return
+
+  os.chdir(HOMEBMDIR)
+  targetpath = '%s/%s.games.%s.tar' % (GAMESDIR, COMMITID, timestamp)
+  os.system('tar cf %s ./output' % (targetpath))
+  os.system('bzip2 %s' % (targetpath))
+
+def cleanup_generated_games():
+  if ARGS.test_output: return
+  os.chdir(HOMEBMDIR)
   os.system('rm ./output/*')
-  os.chdir(SRCDIR)
 
 def log(message):
   LOGF.write('%s: %s\n' % (
@@ -294,7 +312,12 @@ while True:
     skip_init_new_games = False
   else:
     log("Testing new games")
-    test_new_games()
+    timestamp = datetime.datetime.now().strftime('%Y%m%d.%H%M%S')
+    generate_new_games(timestamp)
+    replay_generated_games(timestamp)
+    archive_generated_games(timestamp)
+    cleanup_generated_games()
+    if ARGS.test_output: sys.exit(0)
   nextfile = find_next_file(state)
   if nextfile:
     print "Testing %s..." % nextfile