Merge pull request #255 from simonsobs/develop

Release v0.9.1

.github/workflows/deploy.yml

@@ -137,6 +137,6 @@ jobs:
       run: |
         python3 -m pytest -m 'not (integtest or spt3g)'
 
-    - name: upload to test PyPI
+    - name: upload to PyPI
       run: |
-        python3 -m twine upload --repository testpypi dist/*
+        python3 -m twine upload dist/*

Dockerfile

@@ -34,4 +34,4 @@ RUN pip3 install -r requirements.txt
 COPY . /app/ocs/
 
 # Install ocs
-RUN pip3 install -e .
+RUN pip3 install .

agents/registry/registry.py

@@ -6,8 +6,6 @@
 from collections import defaultdict
 from ocs.ocs_feed import Feed
 
-from ocs.agent.aggregator import Provider
-
 class RegisteredAgent:
     """
         Contains data about registered agents.
@@ -166,7 +164,7 @@ def main(self, session: ocs_agent.OpSession, params):
                     field = f'{addr}_{op_name}'
                     field = field.replace('.', '_')
                     field = field.replace('-', '_')
-                    field = Provider._enforce_field_name_rules(field)
+                    field = Feed.enforce_field_name_rules(field)
                     try:
                         Feed.verify_data_field_string(field)
                     except ValueError as e:

docs/developer/testing.rst

@@ -0,0 +1,37 @@
+Testing
+=======
+
+Writing tests for your OCS Agents is important for the long term maintanence of
+your Agent. Tests allow other developers to contribute to your Agent and easily
+confirm that their changes did not break functionality within the Agent. With
+some setup, tests can also allow you to test your Agent without access to the
+hardware that it controls.
+
+Testing within OCS comes in two forms, unit tests and integration tests.  Unit
+tests test functionality of the Agent code directly, without running the Agent
+itself (or any supporting parts, such as the crossbar server, or a piece of
+hardware to connect to.)
+
+Integration tests run a small OCS network, starting up the crossbar server,
+your Agent, and any supporting programs that your Agent might need (for
+instance, a program accepting serial connections for you Agent to connect to).
+As a result, integration tests are more involved than unit tests, requiring
+more setup and thus taking longer to execute.
+
+Both types of testing can be important for fully testing the functionality of
+your Agent.
+
+Running Tests
+-------------
+
+.. include:: ../../tests/README.rst
+    :start-line: 2
+
+Testing API
+-----------
+
+This section details the helper functions within OCS for assisting with testing
+your Agents.
+
+.. automodule:: ocs.testing
+    :members:

docs/index.rst

@@ -48,6 +48,7 @@ write OCS Agents or Clients.
     developer/clients
     developer/data
     developer/web
+    developer/testing
 
 
 .. toctree::

ocs/agent/aggregator.py

@@ -8,7 +8,7 @@
 import txaio
 txaio.use_twisted()
 
-from ocs import ocs_feed
+from ocs.ocs_feed import Block, Feed
 
 from spt3g import core
 import so3g
@@ -222,7 +222,7 @@ def _verify_provider_data(self, data):
         for block_name, block_dict in data.items():
             for field_name, field_values in block_dict['data'].items():
                 try:
-                    ocs_feed.Feed.verify_data_field_string(field_name)
+                    Feed.verify_data_field_string(field_name)
                 except ValueError:
                     self.log.error("data field name '{field}' is " +
                                    "invalid, removing invalid characters.",
@@ -231,63 +231,12 @@ def _verify_provider_data(self, data):
 
         return verified
 
-    @staticmethod
-    def _enforce_field_name_rules(field_name):
-        """Enforce naming rules for field names.
-
-        A valid name:
-
-        * contains only letters (a-z, A-Z; case sensitive), decimal digits (0-9), and the
-          underscore (_).
-        * begins with a letter, or with any number of underscores followed by a letter.
-        * is at least one, but no more than 255, character(s) long.
-
-        Args:
-            field_name (str):
-                Field name string to check and modify if needed.
-
-        Returns:
-            str: New field name, meeting all above rules. Note this isn't
-                 guarenteed to not collide with other field names passed
-                 through this method, and that should be checked.
-
-        """
-        # check for empty string
-        if field_name == "":
-            new_field_name = "invalid_field"
-        else:
-            new_field_name = field_name
-
-        # replace spaces with underscores
-        new_field_name = new_field_name.replace(' ', '_')
-
-        # replace invalid characters
-        new_field_name = re.sub('[^a-zA-Z0-9_]', '', new_field_name)
-
-        # grab leading underscores
-        underscore_search = re.compile('^_*')
-        underscores = underscore_search.search(new_field_name).group()
-
-        # remove leading underscores
-        new_field_name = re.sub('^_*', '', new_field_name)
-
-        # remove leading non-letters
-        new_field_name = re.sub('^[^a-zA-Z]*', '', new_field_name)
-
-        # add underscores back
-        new_field_name = underscores + new_field_name
-
-        # limit to 255 characters
-        new_field_name = new_field_name[:255]
-
-        return new_field_name
-
     @staticmethod
     def _check_for_duplicate_names(field_name, name_list):
         """Check name_list for matching field names and modify field_name if
         matches are found.
 
-        The results of Provider._enforce_field_name_rules() are not guarenteed
+        The results of ocs_feed.Feed.enforce_field_name_rules() are not guarenteed
         to be unique. This method will check field_name against a list of
         existing field names and try to append '_N', with N being a zero padded
         integer up to 99. Longer integers, though not expected to see use, are
@@ -341,11 +290,11 @@ def _rebuild_invalid_data(self, data):
                     new_data[block_name]['data'] = {}
                     new_field_names = []
                     for field_name, field_values in block_dict['data'].items():
-                        new_field_name = Provider._enforce_field_name_rules(field_name)
+                        new_field_name = Feed.enforce_field_name_rules(field_name)
 
                         # Catch instance where rule enforcement strips all characters
                         if not new_field_name:
-                            new_field_name = Provider._enforce_field_name_rules("invalid_field_" + field_name)
+                            new_field_name = Feed.enforce_field_name_rules("invalid_field_" + field_name)
 
                         new_field_name = Provider._check_for_duplicate_names(new_field_name,
                                                                              new_field_names)
@@ -402,7 +351,7 @@ def save_to_block(self, data):
             try:
                 b = self.blocks[key]
             except KeyError:
-                self.blocks[key] = ocs_feed.Block(
+                self.blocks[key] = Block(
                     key, block['data'].keys(),
                 )
                 b = self.blocks[key]

ocs/ocs_feed.py

@@ -325,3 +325,54 @@ def verify_data_field_string(field):
                              "exceeds the valid length of 255 characters.")
 
         return True
+
+    @staticmethod
+    def enforce_field_name_rules(field_name):
+        """Enforce naming rules for field names.
+
+        A valid name:
+
+        * contains only letters (a-z, A-Z; case sensitive), decimal digits (0-9), and the
+          underscore (_).
+        * begins with a letter, or with any number of underscores followed by a letter.
+        * is at least one, but no more than 255, character(s) long.
+
+        Args:
+            field_name (str):
+                Field name string to check and modify if needed.
+
+        Returns:
+            str: New field name, meeting all above rules. Note this isn't
+                 guarenteed to not collide with other field names passed
+                 through this method, and that should be checked.
+
+        """
+        # check for empty string
+        if field_name == "":
+            new_field_name = "invalid_field"
+        else:
+            new_field_name = field_name
+
+        # replace spaces with underscores
+        new_field_name = new_field_name.replace(' ', '_')
+
+        # replace invalid characters
+        new_field_name = re.sub('[^a-zA-Z0-9_]', '', new_field_name)
+
+        # grab leading underscores
+        underscore_search = re.compile('^_*')
+        underscores = underscore_search.search(new_field_name).group()
+
+        # remove leading underscores
+        new_field_name = re.sub('^_*', '', new_field_name)
+
+        # remove leading non-letters
+        new_field_name = re.sub('^[^a-zA-Z]*', '', new_field_name)
+
+        # add underscores back
+        new_field_name = underscores + new_field_name
+
+        # limit to 255 characters
+        new_field_name = new_field_name[:255]
+
+        return new_field_name

ocs/testing.py

@@ -0,0 +1,126 @@
+import os
+import time
+import pytest
+import signal
+import subprocess
+import coverage.data
+import urllib.request
+
+from urllib.error import URLError
+
+from ocs.ocs_client import OCSClient
+
+
+def create_agent_runner_fixture(agent_path, agent_name, args=None):
+    """Create a pytest fixture for running a given OCS Agent.
+
+    Parameters:
+        agent_path (str): Relative path to Agent,
+            i.e. '../agents/fake_data/fake_data_agent.py'
+        agent_name (str): Short, unique name for the agent
+        args (list): Additional CLI arguments to add when starting the Agent
+
+    """
+    @pytest.fixture()
+    def run_agent(cov):
+        env = os.environ.copy()
+        env['COVERAGE_FILE'] = f'.coverage.agent.{agent_name}'
+        env['OCS_CONFIG_DIR'] = os.getcwd()
+        cmd = ['coverage', 'run',
+               '--rcfile=./.coveragerc',
+               agent_path,
+               '--site-file',
+               './default.yaml']
+        if args is not None:
+            cmd.extend(args)
+        agentproc = subprocess.Popen(cmd,
+                                     env=env,
+                                     stdout=subprocess.PIPE,
+                                     stderr=subprocess.PIPE,
+                                     preexec_fn=os.setsid)
+
+        # wait for Agent to startup
+        time.sleep(1)
+
+        yield
+
+        # shutdown Agent
+        agentproc.send_signal(signal.SIGINT)
+        time.sleep(1)
+
+        # report coverage
+        agentcov = coverage.data.CoverageData(
+            basename=f'.coverage.agent.{agent_name}')
+        agentcov.read()
+        # protect against missing --cov flag
+        if cov is not None:
+            cov.get_data().update(agentcov)
+
+    return run_agent
+
+
+def create_client_fixture(instance_id, timeout=30):
+    """Create the fixture that provides tests a Client object.
+
+    Parameters:
+        instance_id (str): Agent instance-id to connect the Client to
+        timeout (int): Approximate timeout in seconds for the connection.
+            Connection attempts will be made X times, with a 1 second pause
+            between attempts. This is useful if it takes some time for the
+            Agent to start accepting connections, which varies depending on the
+            Agent.
+
+    """
+    @pytest.fixture()
+    def client_fixture():
+        # Set the OCS_CONFIG_DIR so we read the local default.yaml file
+        os.environ['OCS_CONFIG_DIR'] = os.getcwd()
+        print(os.environ['OCS_CONFIG_DIR'])
+        attempts = 0
+
+        while attempts < timeout:
+            try:
+                client = OCSClient(instance_id)
+                break
+            except RuntimeError as e:
+                print(f"Caught error: {e}")
+                print("Attempting to reconnect.")
+
+            time.sleep(1)
+            attempts += 1
+
+        return client
+
+    return client_fixture
+
+
+def check_crossbar_connection(port=18001, interval=5, max_attempts=6):
+    """Check that the crossbar server is up and available for an Agent to
+    connect to.
+
+    Parameters:
+        port (int): Port the crossbar server is configured to run on for
+            testing.
+        interval (float): Amount of time in seconds to wait between checks.
+        max_attempts (int): Maximum number of attempts before giving up.
+
+    Notes:
+        For this check to work the crossbar server needs the `Node Info Service
+        <https://crossbar.io/docs/Node-Info-Service/>`_ running at the path
+        /info.
+
+    """
+    attempts = 0
+
+    while attempts < max_attempts:
+        try:
+            url = f"http://localhost:{port}/info"
+            code = urllib.request.urlopen(url).getcode()
+        except (URLError, ConnectionResetError):
+            print("Crossbar server not online yet, waiting 5 seconds.")
+            time.sleep(interval)
+
+        attempts += 1
+
+    assert code == 200
+    print("Crossbar server online.")