Merge pull request #1855 from OpenC3/py_ast

Improve python instrumentation

.github/workflows/api_tests.yml

@@ -60,7 +60,7 @@ jobs:
           flags: ruby-api # See codecov.yml
           token: ${{ secrets.CODECOV_TOKEN }}
 
-  script-runner-api:
+  script-runner-api-ruby:
     if: ${{ github.actor != 'dependabot[bot]' }}
     runs-on: ubuntu-latest
     strategy:
@@ -90,3 +90,37 @@ jobs:
           directory: openc3-cosmos-script-runner-api/coverage
           flags: ruby-api # See codecov.yml
           token: ${{ secrets.CODECOV_TOKEN }}
+
+  script-runner-api-python:
+    if: ${{ github.actor != 'dependabot[bot]' }}
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: ["3.10", "3.11"]
+
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install -r requirements.txt
+          pip install -r requirements-dev.txt
+        working-directory: openc3/python
+      - name: Lint with ruff
+        run: |
+          ruff --config=../openc3/python/pyproject.toml --format=github scripts/*.py
+        working-directory: openc3-cosmos-script-runner-api
+      - name: Run unit tests
+        run: |
+          coverage run -m pytest ./test/
+          coverage xml -i
+        working-directory: openc3-cosmos-script-runner-api
+      - uses: codecov/codecov-action@v5
+        with:
+          working-directory: openc3/python
+          flags: python # See codecov.yml
+          token: ${{ secrets.CODECOV_TOKEN }}

openc3-cosmos-script-runner-api/scripts/run_script.py

@@ -176,7 +176,7 @@ def run_script_log(id, message, color="BLACK", message_log=True):
                             | "open_file_dialog"
                             | "open_files_dialog"
                         ):
-                            if running_script.prompt_id != None:
+                            if running_script.prompt_id is not None:
                                 if (
                                     "prompt_id" in parsed_cmd
                                     and running_script.prompt_id
@@ -256,7 +256,7 @@ def run_script_log(id, message, color="BLACK", message_log=True):
                     run_script_log(
                         id, f"ERROR: Script command not handled: {msg['data']}", "RED"
                     )
-except Exception as err:
+except Exception:
     tb = traceback.format_exc()
     run_script_log(id, tb, "RED")
 finally:

openc3-cosmos-script-runner-api/scripts/running_script.py

@@ -18,7 +18,6 @@
 from openc3.utilities.string import build_timestamped_filename
 from openc3.utilities.bucket_utilities import BucketUtilities
 from openc3.script.storage import _get_storage_file
-import re
 import linecache
 
 SCRIPT_API = 'script-api'
@@ -439,7 +438,11 @@ def unique_filename(self):
             return "Untitled" + str(RunningScript.id)
 
     def stop_message_log(self):
-        metadata = {"id": self.id, "user": self.details["user"], "scriptname": self.unique_filename()}
+        metadata = {
+            "id": self.id,
+            "user": self.details["user"],
+            "scriptname": self.unique_filename(),
+        }
         if RunningScript.my_message_log:
             RunningScript.my_message_log.stop(True, metadata=metadata)
         RunningScript.my_message_log = None
@@ -452,7 +455,7 @@ def set_filename(self, filename):
 
         # Deal with breakpoints created under the previous filename.
         bkpt_filename = self.unique_filename()
-        if not bkpt_filename in RunningScript.breakpoints:
+        if bkpt_filename not in RunningScript.breakpoints:
             RunningScript.breakpoints[bkpt_filename] = RunningScript.breakpoints[
                 self.filename
             ]
@@ -493,6 +496,7 @@ def instrument_script(cls, text, filename):
 
         parsed = ast.parse(text)
         tree = ScriptInstrumentor(filename).visit(parsed)
+        # Normal Python code is run with mode='exec' whose root is ast.Module
         result = compile(tree, filename=filename, mode="exec")
         return result
 
@@ -540,7 +544,7 @@ def exception_instrumentation(self, filename, line_number):
         if (
             exc_type == StopScript
             or exc_type == SkipScript
-            or exc_type == SkipTestCase # DEPRECATED but still valid
+            or exc_type == SkipTestCase  # DEPRECATED but still valid
             or not self.use_instrumentation
         ):
             raise exc_value
@@ -598,20 +602,20 @@ def debug(self, debug_text):
 
     @classmethod
     def set_breakpoint(cls, filename, line_number):
-        if not filename in cls.breakpoints:
+        if filename not in cls.breakpoints:
             cls.breakpoints[filename] = {}
         cls.breakpoints[filename][line_number] = True
 
     @classmethod
     def clear_breakpoint(cls, filename, line_number):
-        if not filename in cls.breakpoints:
+        if filename not in cls.breakpoints:
             cls.breakpoints[filename] = {}
         if line_number in cls.breakpoints[filename]:
             del cls.breakpoints[filename][line_number]
 
     @classmethod
     def clear_breakpoints(cls, filename=None):
-        if filename == None or filename == "":
+        if filename is None or filename == "":
             cls.breakpoints = {}
         else:
             if filename in cls.breakpoints:
@@ -680,7 +684,7 @@ def handle_output_io(self, filename=None, line_number=None):
                         out_line = json_hash["log"]
                     if "message" in json_hash:
                         out_line = json_hash["message"]
-                except:
+                except Exception:
                     # Regular output
                     pass
 
@@ -723,10 +727,6 @@ def handle_output_io(self, filename=None, line_number=None):
             # Add to the message log
             self.message_log().write(lines_to_write)
 
-    def graceful_kill(self):
-        # Just to avoid warning
-        pass
-
     def wait_for_go_or_stop(self, error=None, prompt=None):
         count = -1
         self.go = False
@@ -1072,7 +1072,7 @@ def run_text(
     def handle_potential_tab_change(self, filename):
         # Make sure the correct file is shown in script runner
         if self.current_file != filename:
-            if not filename in self.call_stack:
+            if filename not in self.call_stack:
                 self.call_stack.append(filename)
                 self.load_file_into_script(filename)
             self.current_file = filename
@@ -1133,10 +1133,13 @@ def handle_exception(
             self.mark_error()
             self.wait_for_go_or_stop_or_retry(exc_value)
 
+        # See script_instrumentor.py for how retry is used
         if self.retry_needed:
             self.retry_needed = False
+            # Return True to the instrumented code to retry the line
             return True
         else:
+            # Return False to the instrumented code to break the while loop
             return False
 
     def load_file_into_script(self, filename):