process replay: automatically push refs on fail (#24414)

* test failure()

* let's just change a tune here

* debug

revert

* debug

* use current commit, not ref_commit

fix

* need to figure out better place for this

* quick test

* test without upload

* temp

* use azure token

* fixes

* shouldn't need this

* debug

* debug

* not getting anything?

* does this mean nothing gets envvars?

* add azure token to docker environment variables

* quote

* move back

* clean up a bit

* more clean up

* like this sorting better

* replace flags with CI and clean up

* test FULL_TEST and minimalize diff a bit

* now test all

* revert tests comments

* remove flags

* revert this

revert this

* now make it fail

* now update ref_commit to last commit (make sure we can re-start this test if we commit before last one finishes uploading)

* fix

fix

fix

fix

* bad commit

* why is it not throwing an exception?

* debug

* URLFile returns empty bytes if using cache and remote file doesn't exist

* we always need to download anyway

* debug...

* duh, wrong file. but neither should have it

* add that back and just check explicitly

* check both

* clean up and make a diff

* stylize

* see if this is a better diff on files changed

* update refs

* revert changes

* only for owners or members

* if we have token access

* if we have token access

* if we have token access

* move up

* clean up

* revert

* move update refs to test_processes

* clean up

* update messages

* update msg

* update README and delete update_refs

* this isn't possible to reach anymore

* fix readme

* better help message

better help message

better help message

* only show basename when uploading, only if failed to find

* test diff

* fix printing old ref commit

* change to using

* update refs

* Revert "update refs"

This reverts commit 2e352a736a.

* revert

* ref refers to reference commit/logs, cur refers to current logs/commit (future ref)

* like for better

* Apply suggestions from code review

Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>

* Update selfdrive/test/process_replay/test_processes.py

* every time lol

Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>

.github/workflows/selfdrive_tests.yaml

@@ -8,6 +8,7 @@ on:
 env:
   BASE_IMAGE: openpilot-base
   DOCKER_REGISTRY: ghcr.io/commaai
+  AZURE_TOKEN: ${{ secrets.AZURE_COMMADATACI_OPENPILOTCI_TOKEN }}
 
   DOCKER_LOGIN: docker login ghcr.io -u adeebshihadeh -p ${{ secrets.CONTAINER_TOKEN }}
   BUILD: |
@@ -316,6 +317,11 @@ jobs:
         ${{ env.RUN }} "scons -j$(nproc) && \
                         FILEREADER_CACHE=1 CI=1 coverage run selfdrive/test/process_replay/test_processes.py && \
                         coverage xml"
+    - name: Upload reference logs
+      if: ${{ failure() && github.event_name == 'pull_request' && github.repository == 'commaai/openpilot' && env.AZURE_TOKEN != '' }}
+      run: |
+        ${{ env.RUN }} "scons -j$(nproc) && \
+                        CI=1 AZURE_TOKEN='$AZURE_TOKEN' python selfdrive/test/process_replay/test_processes.py --upload-only"
     - name: "Upload coverage to Codecov"
       uses: codecov/codecov-action@v2
     - name: Print diff

selfdrive/test/openpilotci.py

@@ -4,17 +4,18 @@ import sys
 import subprocess
 
 BASE_URL = "https://commadataci.blob.core.windows.net/openpilotci/"
-
 TOKEN_PATH = "/data/azure_token"
 
+
 def get_url(route_name, segment_num, log_type="rlog"):
   ext = "hevc" if log_type.endswith('camera') else "bz2"
   return BASE_URL + f"{route_name.replace('|', '/')}/{segment_num}/{log_type}.{ext}"
 
+
 def upload_file(path, name):
   from azure.storage.blob import BlockBlobService  # pylint: disable=import-error
 
-  sas_token = None
+  sas_token = os.environ.get("AZURE_TOKEN", None)
   if os.path.isfile(TOKEN_PATH):
     sas_token = open(TOKEN_PATH).read().strip()
 
@@ -25,6 +26,7 @@ def upload_file(path, name):
   service.create_blob_from_path("openpilotci", name, path)
   return "https://commadataci.blob.core.windows.net/openpilotci/" + name
 
+
 if __name__ == "__main__":
   for f in sys.argv[1:]:
     name = os.path.basename(f)

selfdrive/test/process_replay/README.md

@@ -1,4 +1,4 @@
-# process replay
+# Process replay
 
 Process replay is a regression test designed to identify any changes in the output of a process. This test replays a segment through individual processes and compares the output to a known good replay. Each make is represented in the test with a segment.
 
@@ -12,14 +12,34 @@ Currently the following processes are tested:
 * radard
 * plannerd
 * calibrationd
+* dmonitoringd
+* locationd
+* paramsd
 * ubloxd
 
+### Usage
+```
+Usage: test_processes.py [-h] [--whitelist-procs PROCS] [--whitelist-cars CARS] [--blacklist-procs PROCS]
+                         [--blacklist-cars CARS] [--ignore-fields FIELDS] [--ignore-msgs MSGS] [--update-refs] [--upload-only]
+Regression test to identify changes in a process's output
+optional arguments:
+  -h, --help            show this help message and exit
+  --whitelist-procs PROCS               Whitelist given processes from the test (e.g. controlsd)
+  --whitelist-cars WHITELIST_CARS       Whitelist given cars from the test (e.g. HONDA)
+  --blacklist-procs BLACKLIST_PROCS     Blacklist given processes from the test (e.g. controlsd)
+  --blacklist-cars BLACKLIST_CARS       Blacklist given cars from the test (e.g. HONDA)
+  --ignore-fields IGNORE_FIELDS         Extra fields or msgs to ignore (e.g. carState.events)
+  --ignore-msgs IGNORE_MSGS             Msgs to ignore (e.g. carEvents)
+  --update-refs                         Updates reference logs using current commit
+  --upload-only                         Skips testing processes and uploads logs from previous test run
+```
+
 ## Forks
 
-openpilot forks can use this test with their own reference logs
+openpilot forks can use this test with their own reference logs, by default `test_proccess.py` saves logs locally.
 
 To generate new logs:
 
-`./update_refs.py --no-upload`
+`./test_processes.py`
 
-Then, check in the new logs using git-lfs. Make sure to also include the updated `ref_commit` file.
+Then, check in the new logs using git-lfs. Make sure to also update the `ref_commit` file to the current commit.

selfdrive/test/process_replay/process_replay.py

@@ -24,6 +24,7 @@ from selfdrive.manager.process_config import managed_processes
 NUMPY_TOLERANCE = 1e-7
 CI = "CI" in os.environ
 TIMEOUT = 15
+PROC_REPLAY_DIR = os.path.dirname(os.path.abspath(__file__))
 
 ProcessConfig = namedtuple('ProcessConfig', ['proc_name', 'pub_sub', 'ignore', 'init_callback', 'should_recv_callback', 'tolerance', 'fake_pubsubmaster', 'submaster_config'], defaults=({},))
 

selfdrive/test/process_replay/test_processes.py

@@ -5,9 +5,10 @@ import sys
 from typing import Any
 
 from selfdrive.car.car_helpers import interface_names
-from selfdrive.test.openpilotci import get_url
-from selfdrive.test.process_replay.compare_logs import compare_logs
-from selfdrive.test.process_replay.process_replay import CONFIGS, replay_process, check_enabled
+from selfdrive.test.openpilotci import get_url, upload_file
+from selfdrive.test.process_replay.compare_logs import compare_logs, save_log
+from selfdrive.test.process_replay.process_replay import CONFIGS, PROC_REPLAY_DIR, check_enabled, replay_process
+from selfdrive.version import get_commit
 from tools.lib.logreader import LogReader
 
 
@@ -50,32 +51,31 @@ segments = [
 excluded_interfaces = ["mock", "ford", "mazda", "tesla"]
 
 BASE_URL = "https://commadataci.blob.core.windows.net/openpilotci/"
+REF_COMMIT_FN = os.path.join(PROC_REPLAY_DIR, "ref_commit")
 
-# run the full test (including checks) when no args given
-FULL_TEST = len(sys.argv) <= 1
 
-
-def test_process(cfg, lr, cmp_log_fn, ignore_fields=None, ignore_msgs=None):
+def test_process(cfg, lr, ref_log_fn, ignore_fields=None, ignore_msgs=None):
   if ignore_fields is None:
     ignore_fields = []
   if ignore_msgs is None:
     ignore_msgs = []
 
-  cmp_log_path = cmp_log_fn if os.path.exists(cmp_log_fn) else BASE_URL + os.path.basename(cmp_log_fn)
-  cmp_log_msgs = list(LogReader(cmp_log_path))
+  ref_log_path = ref_log_fn if os.path.exists(ref_log_fn) else BASE_URL + os.path.basename(ref_log_fn)
+  ref_log_msgs = list(LogReader(ref_log_path))
 
   log_msgs = replay_process(cfg, lr)
 
   # check to make sure openpilot is engaged in the route
   if cfg.proc_name == "controlsd":
     if not check_enabled(log_msgs):
-      segment = cmp_log_fn.split("/")[-1].split("_")[0]
+      segment = ref_log_fn.split("/")[-1].split("_")[0]
       raise Exception(f"Route never enabled: {segment}")
 
   try:
-    return compare_logs(cmp_log_msgs, log_msgs, ignore_fields+cfg.ignore, ignore_msgs, cfg.tolerance)
+    return compare_logs(ref_log_msgs, log_msgs, ignore_fields + cfg.ignore, ignore_msgs, cfg.tolerance), log_msgs
   except Exception as e:
-    return str(e)
+    return str(e), log_msgs
+
 
 def format_diff(results, ref_commit):
   diff1, diff2 = "", ""
@@ -106,47 +106,57 @@ def format_diff(results, ref_commit):
         failed = True
   return diff1, diff2, failed
 
-if __name__ == "__main__":
 
+if __name__ == "__main__":
   parser = argparse.ArgumentParser(description="Regression test to identify changes in a process's output")
 
   # whitelist has precedence over blacklist in case both are defined
   parser.add_argument("--whitelist-procs", type=str, nargs="*", default=[],
-                        help="Whitelist given processes from the test (e.g. controlsd)")
+                      help="Whitelist given processes from the test (e.g. controlsd)")
   parser.add_argument("--whitelist-cars", type=str, nargs="*", default=[],
-                        help="Whitelist given cars from the test (e.g. HONDA)")
+                      help="Whitelist given cars from the test (e.g. HONDA)")
   parser.add_argument("--blacklist-procs", type=str, nargs="*", default=[],
-                        help="Blacklist given processes from the test (e.g. controlsd)")
+                      help="Blacklist given processes from the test (e.g. controlsd)")
   parser.add_argument("--blacklist-cars", type=str, nargs="*", default=[],
-                        help="Blacklist given cars from the test (e.g. HONDA)")
+                      help="Blacklist given cars from the test (e.g. HONDA)")
   parser.add_argument("--ignore-fields", type=str, nargs="*", default=[],
-                        help="Extra fields or msgs to ignore (e.g. carState.events)")
+                      help="Extra fields or msgs to ignore (e.g. carState.events)")
   parser.add_argument("--ignore-msgs", type=str, nargs="*", default=[],
-                        help="Msgs to ignore (e.g. carEvents)")
+                      help="Msgs to ignore (e.g. carEvents)")
+  parser.add_argument("--update-refs", action="store_true",
+                      help="Updates reference logs using current commit")
+  parser.add_argument("--upload-only", action="store_true",
+                      help="Skips testing processes and uploads logs from previous test run")
   args = parser.parse_args()
 
-  cars_whitelisted = len(args.whitelist_cars) > 0
-  procs_whitelisted = len(args.whitelist_procs) > 0
+  full_test = all(len(x) == 0 for x in (args.whitelist_procs, args.whitelist_cars, args.blacklist_procs, args.blacklist_cars, args.ignore_fields, args.ignore_msgs))
+  upload = args.update_refs or args.upload_only
+
+  if upload:
+    assert full_test, "Need to run full test when updating refs"
 
-  process_replay_dir = os.path.dirname(os.path.abspath(__file__))
   try:
-    ref_commit = open(os.path.join(process_replay_dir, "ref_commit")).read().strip()
+    ref_commit = open(REF_COMMIT_FN).read().strip()
   except FileNotFoundError:
-    print("couldn't find reference commit")
+    print("Couldn't find reference commit")
     sys.exit(1)
 
+  cur_commit = get_commit()
+  if cur_commit is None:
+    raise Exception("Couldn't get current commit")
+
   print(f"***** testing against commit {ref_commit} *****")
 
   # check to make sure all car brands are tested
-  if FULL_TEST:
+  if full_test:
     tested_cars = {c.lower() for c, _ in segments}
     untested = (set(interface_names) - set(excluded_interfaces)) - tested_cars
     assert len(untested) == 0, f"Cars missing routes: {str(untested)}"
 
   results: Any = {}
   for car_brand, segment in segments:
-    if (cars_whitelisted and car_brand.upper() not in args.whitelist_cars) or \
-       (not cars_whitelisted and car_brand.upper() in args.blacklist_cars):
+    if (len(args.whitelist_cars) and car_brand.upper() not in args.whitelist_cars) or \
+       (not len(args.whitelist_cars) and car_brand.upper() in args.blacklist_cars):
       continue
 
     print(f"***** testing route segment {segment} *****\n")
@@ -157,23 +167,40 @@ if __name__ == "__main__":
     lr = LogReader(get_url(r, n))
 
     for cfg in CONFIGS:
-      if (procs_whitelisted and cfg.proc_name not in args.whitelist_procs) or \
-         (not procs_whitelisted and cfg.proc_name in args.blacklist_procs):
+      if (len(args.whitelist_procs) and cfg.proc_name not in args.whitelist_procs) or \
+         (not len(args.whitelist_procs) and cfg.proc_name in args.blacklist_procs):
         continue
 
-      cmp_log_fn = os.path.join(process_replay_dir, f"{segment}_{cfg.proc_name}_{ref_commit}.bz2")
-      results[segment][cfg.proc_name] = test_process(cfg, lr, cmp_log_fn, args.ignore_fields, args.ignore_msgs)
+      cur_log_fn = os.path.join(PROC_REPLAY_DIR, f"{segment}_{cfg.proc_name}_{cur_commit}.bz2")
+      if not args.upload_only:
+        ref_log_fn = os.path.join(PROC_REPLAY_DIR, f"{segment}_{cfg.proc_name}_{ref_commit}.bz2")
+        results[segment][cfg.proc_name], log_msgs = test_process(cfg, lr, ref_log_fn, args.ignore_fields, args.ignore_msgs)
+
+        # save logs so we can upload when updating refs
+        save_log(cur_log_fn, log_msgs)
+
+      if upload:
+        print(f'Uploading: {os.path.basename(cur_log_fn)}')
+        assert os.path.exists(cur_log_fn), f"Cannot find log to upload: {cur_log_fn}"
+        upload_file(cur_log_fn, os.path.basename(cur_log_fn))
+        os.remove(cur_log_fn)
 
   diff1, diff2, failed = format_diff(results, ref_commit)
-  with open(os.path.join(process_replay_dir, "diff.txt"), "w") as f:
+  with open(os.path.join(PROC_REPLAY_DIR, "diff.txt"), "w") as f:
     f.write(diff2)
   print(diff1)
 
   if failed:
     print("TEST FAILED")
-    print("\n\nTo update the reference logs for this test run:")
-    print("./update_refs.py")
+    if not upload:
+      print("\n\nTo push the new reference logs for this commit run:")
+      print("./test_processes.py --upload-only")
   else:
     print("TEST SUCCEEDED")
 
+  if upload:
+    with open(REF_COMMIT_FN, "w") as f:
+      f.write(cur_commit)
+    print(f"\n\nUpdated reference logs for commit: {cur_commit}")
+
   sys.exit(int(failed))

selfdrive/test/process_replay/update_refs.py

@@ -1,38 +0,0 @@
-#!/usr/bin/env python3
-import os
-import sys
-
-from selfdrive.test.openpilotci import upload_file, get_url
-from selfdrive.test.process_replay.compare_logs import save_log
-from selfdrive.test.process_replay.process_replay import replay_process, CONFIGS
-from selfdrive.test.process_replay.test_processes import segments
-from selfdrive.version import get_commit
-from tools.lib.logreader import LogReader
-
-if __name__ == "__main__":
-
-  no_upload = "--no-upload" in sys.argv
-
-  process_replay_dir = os.path.dirname(os.path.abspath(__file__))
-  ref_commit_fn = os.path.join(process_replay_dir, "ref_commit")
-
-  ref_commit = get_commit()
-  if ref_commit is None:
-    raise Exception("couldn't get ref commit")
-  with open(ref_commit_fn, "w") as f:
-    f.write(ref_commit)
-
-  for car_brand, segment in segments:
-    r, n = segment.rsplit("--", 1)
-    lr = LogReader(get_url(r, n))
-
-    for cfg in CONFIGS:
-      log_msgs = replay_process(cfg, lr)
-      log_fn = os.path.join(process_replay_dir, f"{segment}_{cfg.proc_name}_{ref_commit}.bz2")
-      save_log(log_fn, log_msgs)
-
-      if not no_upload:
-        upload_file(log_fn, os.path.basename(log_fn))
-        os.remove(log_fn)
-
-  print("done")