Tests/add plugin pytests (#15)

* Lets trigger tests

* Yucky dependency installation

* Skip flakey for setup

* Lint

* Coverage

* Small whoopsie in cov call

* Change trigger for code cov

* Fix cov

* Start testing, write test plan

* Split up tests config and announcing

* Fix linting and tests

* Change pytest

* Adjust cov file

* Revert unknown option

* Swap lint, fix some

* Small changes

* Fix folder name

* Fix cov

* Try fix

* Remove some

* Clean up

* Clean more

* Verbose

* Fix

* Test

* Fix tests

* Yuck

* Test further with mocks

* Fix test, improve coverage slightly

* Add config key assertions

* Flakey

* More coverage of config functionality

* Added octofarm version check test with mock

* Add update state check

* Cover more on config

* Mock settings

* Validation test

* Avoid writing files during test, keep cov

* Access_token and Announce flow

* Finalize tests

* Cleanup a bit

* Adjust changelog

* Comment

.coveragerc

@@ -0,0 +1,2 @@
+[run]
+omit = tests/*

.github/workflows/pytest.yml

@@ -0,0 +1,38 @@
+name: Python package
+
+on: [ pull_request ]
+
+jobs:
+  build:
+
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: [ 3.7, 3.8 ]
+
+    steps:
+      - uses: actions/checkout@v2
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v2
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install flake8 pytest pytest-cov flask octoprint requests
+          if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
+      - name: Test with pytest and pytest-cov
+        run: |
+          pytest --cache-clear --verbose --cov=octofarm_companion
+      - name: Build coverage file
+        run: |
+          pytest --cache-clear --cov=octofarm_companion tests/ > pytest-coverage.txt
+      - name: Lint with flake8
+        run: |
+          # stop the build if there are Python syntax errors or undefined names
+          flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
+          # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
+          flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
+
+      - name: Comment coverage
+        uses: coroo/[email protected]

.gitignore

@@ -134,3 +134,10 @@ dmypy.json
 
 # Pyre type checker
 .pyre/
+
+tests/test_data/backup_excluded_data.json
+test_data/backup_excluded_data.json
+
+pytest-coverage.txt
+
+test

CHANGELOG.md

@@ -2,12 +2,24 @@
 
 All notable changes to this project will be documented in this file.
 
+## [0.1.0-rc1-build3]
+
+### Added
+    - Test configuration and announcing with coverage up to 92%.
+
+### Changed
+
+### Removed
+
+### Fixed
+    - Abstracted state variable and accessor keys
+
 
 ## [0.1.0-rc1-build2]
 
 ### Added
- 
-### Changed 
+
+### Changed
 
 ### Removed
 

octofarm_companion/__init__.py

@@ -1,22 +1,20 @@
 # -*- coding: utf-8 -*-
 from __future__ import absolute_import, unicode_literals
 
-import flask
 import io
 import json
 import os
 import uuid
 from datetime import datetime
 from urllib.parse import urljoin
 
+import flask
 import octoprint.plugin
 import requests
+from flask import request
 from octoprint.util import RepeatedTimer
 
-from octoprint.server import NO_CONTENT
-from octoprint.server.util.flask import (
-    no_firstrun_access
-)
+from octofarm_companion.constants import Errors, State, Config, Keys
 
 
 def is_docker():
@@ -45,21 +43,20 @@ def __init__(self):
         self._ping_worker = None
         # device UUID and OIDC opaque access_token + metadata
         self._persisted_data = dict()
-        self._excluded_persistence_data = "backup_excluded_data.json"
         self._excluded_persistence_datapath = None
-        self._state = "boot"
+        self._state = State.BOOT
 
     def on_after_startup(self):
         if self._settings.get(["octofarm_host"]) is None:
-            self._settings.set(["octofarm_host"], "http://127.0.0.1")
+            self._settings.set(["octofarm_host"], Config.default_octofarm_host)
         if self._settings.get(["octofarm_port"]) is None:
-            self._settings.set(["octofarm_port"], 4000)
+            self._settings.set(["octofarm_port"], Config.default_octofarm_port)
         self._get_device_uuid()
         self._start_periodic_check()
 
     def get_excluded_persistence_datapath(self):
         self._excluded_persistence_datapath = os.path.join(self.get_plugin_data_folder(),
-                                                           self._excluded_persistence_data)
+                                                           Config.persisted_data_file)
         return self._excluded_persistence_datapath
 
     def get_template_vars(self):
@@ -84,7 +81,7 @@ def get_settings_defaults(self):
             "device_uuid": None,  # Auto-generated and unique
             "oidc_client_id": None,  # Without adjustment this config value is ALWAYS useless
             "oidc_client_secret": None,  # Without adjustment this config value is ALWAYS useless
-            "ping": 120
+            "ping": Config.default_ping_secs
         }
 
     def get_settings_version(self):
@@ -121,14 +118,16 @@ def _write_new_access_token(self, filepath, at_data):
         self._persisted_data["access_token"] = at_data["access_token"]
         self._persisted_data["expires_in"] = at_data["expires_in"]
         self._persisted_data["requested_at"] = int(datetime.utcnow().timestamp())
-        self._persisted_data["token_type"] = at_data["token_type"]
-        self._persisted_data["scope"] = at_data["scope"]
+        if "token_type" in at_data.keys():
+            self._persisted_data["token_type"] = at_data["token_type"]
+        if "scope" in at_data.keys():
+            self._persisted_data["scope"] = at_data["scope"]
         self._write_persisted_data(filepath)
         self._logger.info("OctoFarm persisted data file was updated (access_token)")
 
     def _write_new_device_uuid(self, filepath):
         persistence_uuid = str(uuid.uuid4())
-        self._persisted_data['persistence_uuid'] = persistence_uuid
+        self._persisted_data[Keys.persistence_uuid_key] = persistence_uuid
         self._write_persisted_data(filepath)
         self._logger.info("OctoFarm persisted data file was updated (device_uuid).")
 
@@ -137,10 +136,10 @@ def _write_persisted_data(self, filepath):
             f.write(json.dumps(self._persisted_data))
 
     def _get_device_uuid(self):
-        device_uuid = self._settings.get(["device_uuid"])
+        device_uuid = self._settings.get([Keys.device_uuid_key])
         if device_uuid is None:
             device_uuid = str(uuid.uuid4())
-            self._settings.set(["device_uuid"], device_uuid)
+            self._settings.set([Keys.device_uuid_key], device_uuid)
             self._settings.save()
         return device_uuid
 
@@ -172,6 +171,8 @@ def _start_periodic_check(self):
                     ping_interval, self._check_octofarm, run_first=True
                 )
                 self._ping_worker.start()
+            else:
+                return self._logger.error(Errors.ping_setting_unset)
 
     def _check_octofarm(self):
         octofarm_host = self._settings.get(["octofarm_host"])
@@ -181,9 +182,9 @@ def _check_octofarm(self):
             base_url = f"{octofarm_host}:{octofarm_port}"
 
             # OIDC client_credentials flow result
-            access_token = self._persisted_data.get('access_token', None)
-            requested_at = self._persisted_data.get('requested_at', None)
-            expires = self._persisted_data.get('expires', None)
+            access_token = self._persisted_data.get("access_token", None)
+            requested_at = self._persisted_data.get("requested_at", None)
+            expires = self._persisted_data.get("expires", None)
 
             # Token expiry check - prone to time desync
             is_expired = None
@@ -196,27 +197,32 @@ def _check_octofarm(self):
             if token_invalid:
                 oidc_client_id = self._settings.get(["oidc_client_id"])
                 oidc_client_secret = self._settings.get(["oidc_client_secret"])
+                self._logger.info("Refreshing access_token as it was expired")
                 success = self._query_access_token(base_url, oidc_client_id, oidc_client_secret)
                 if not success:
+                    self._state = State.CRASHED
                     return False
             else:
-                self._state = "success"
+                # We skip querying the token
+                self._state = State.SUCCESS
+
+            if "access_token" not in self._persisted_data.keys():
+                # Quite unlikely as we'd be crashed
+                raise Exception(Errors.access_token_not_saved)
 
             at = self._persisted_data["access_token"]
-            if at is None:
-                raise Exception(
-                    "Conditional error: 'access_token' was not saved properly. Please report a bug to the plugin developers. Aborting")
 
             self._query_announcement(base_url, at)
 
         else:
-            raise Exception("Configuration error: 'oidc_client_id' or 'oidc_client_secret' not set")
-            self._logger.error("Error connecting to OctoFarm")
+            self._logger.error(Errors.openid_config_unset)
+            self._state = State.CRASHED
+            raise Exception(Errors.config_openid_missing)
 
     def _query_access_token(self, base_url, oidc_client_id, oidc_client_secret):
         if not oidc_client_id or not oidc_client_secret:
             self._logger.error("Configuration error: 'oidc_client_id' or 'oidc_client_secret' not set")
-            self._state = "crash"
+            self._state = State.CRASHED
             return False
 
         at_data = None
@@ -230,41 +236,39 @@ def _query_access_token(self, base_url, oidc_client_id, oidc_client_secret):
             self._logger.info(response.status_code)
             at_data = json.loads(response.text)
         except requests.exceptions.ConnectionError:
-            self._state = "retry"  # TODO apply this with a backoff scheme
+            self._state = State.RETRY  # TODO apply this with a backoff scheme
             self._logger.error("ConnectionError: error sending access_token request to OctoFarm")
         except Exception as e:
-            self._state = "crash"
+            self._state = State.CRASHED
             self._logger.error(
                 "Generic Exception: error requesting access_token request to OctoFarm. Exception: " + str(e))
 
         if at_data is not None:
-            if at_data["access_token"] is None:
+            if "access_token" not in at_data.keys():
                 raise Exception(
                     "Response error: 'access_token' not received. Check your OctoFarm server logs. Aborting")
-            if at_data["expires_in"] is None:
+            if "expires_in" not in at_data.keys() is None:
                 raise Exception("Response error: 'expires_in' not received. Check your OctoFarm server logs. Aborting")
 
             # Saves to file and to this plugin instance self._persistence_data accordingly
             self._write_new_access_token(self.get_excluded_persistence_datapath(), at_data)
-            self._state = "success"
+            self._state = State.SUCCESS
             return True
         else:
-            self._state = "crash"
+            self._state = State.CRASHED
             self._logger.error("Response error: access_token data response was empty. Aborting")
 
     def _query_announcement(self, base_url, access_token):
-        if self._state is not "success" and self._state is not "sleep":
+        if self._state != State.SUCCESS and self._state != State.SLEEP:
             self._logger.error("State error: tried to announce when state was not 'success'")
 
         if base_url is None:
-            self._state = "crash"
-            raise Exception(
-                "The 'base_url' was not provided. Preventing announcement query to OctoFarm")
+            self._state = State.CRASHED
+            raise Exception(Errors.base_url_not_provided)
 
         if len(access_token) < 43:
-            self._state = "crash"
-            raise Exception(
-                "The 'access_token' did not meet the expected length of 43 characters. Preventing announcement query to OctoFarm")
+            self._state = State.CRASHED
+            raise Exception(Errors.access_token_too_short)
 
         # Announced data
         octoprint_port = self._settings.get(["port_override"])
@@ -296,22 +300,27 @@ def _query_announcement(self, base_url, access_token):
             url = urljoin(base_url, octofarm_announce_route)
             response = requests.post(url, headers=headers, json=check_data)
 
-            self._state = "sleep"
+            self._state = State.SLEEP
             self._logger.info(f"Done announcing to OctoFarm server ({response.status_code})")
             self._logger.info(response.text)
         except requests.exceptions.ConnectionError:
-            self._state = "crash"
+            self._state = State.CRASHED
             self._logger.error("ConnectionError: error sending announcement to OctoFarm")
 
-    def additional_excludes_hook(self, excludes, *args, **kwargs):
-        return [self._excluded_persistence_data]
+    def _call_validator_abort(self, key):
+        flask.abort(400, description=f"Expected '{key}' parameter")
+
+    @staticmethod
+    def additional_excludes_hook(excludes, *args, **kwargs):
+        return [Config.persisted_data_file]
 
     @octoprint.plugin.BlueprintPlugin.route("/test_octofarm_connection", methods=["POST"])
-    @no_firstrun_access
     def test_octofarm_connection(self):
-        input = json.loads(flask.request.data)
-        if "url" not in input:
-            flask.abort(400, description="Expected 'url' parameter")
+        input = json.loads(request.data)
+        keys = ["url"]
+        for key in keys:
+            if key not in input:
+                return self._call_validator_abort(key)
 
         proposed_url = input["url"]
         self._logger.info("Testing OctoFarm URL " + proposed_url)
@@ -325,20 +334,17 @@ def test_octofarm_connection(self):
         return version_data
 
     @octoprint.plugin.BlueprintPlugin.route("/test_octofarm_openid", methods=["POST"])
-    @no_firstrun_access
     def test_octofarm_openid(self):
-        input = json.loads(flask.request.data)
-        if not "url" in input:
-            flask.abort(400, description="Expected 'url' parameter")
-        if not "client_id" in input:
-            flask.abort(400, description="Expected 'client_id' parameter")
-        if not "client_secret" in input:
-            flask.abort(400, description="Expected 'client_secret' parameter")
+        input = json.loads(request.data)
+        keys = ["url", "client_id", "client_secret"]
+        for key in keys:
+            if key not in input:
+                return self._call_validator_abort(key)
 
         proposed_url = input["url"]
         oidc_client_id = input["client_id"]
         oidc_client_secret = input["client_secret"]
-        self._query_access_token(proposed_url, oidc_client_id, oidc_client_secret)
+        response = self._query_access_token(proposed_url, oidc_client_id, oidc_client_secret)
 
         self._logger.info("Queried access_token from Octofarm")
 
@@ -348,7 +354,7 @@ def test_octofarm_openid(self):
 
 
 __plugin_name__ = "OctoFarm Companion"
-__plugin_version__ = "0.1.0-rc1-build2"
+__plugin_version__ = "0.1.0-rc1-build3"
 __plugin_description__ = "The OctoFarm companion plugin for OctoPrint"
 __plugin_pythoncompat__ = ">=3,<4"
 

octofarm_companion/constants.py

@@ -0,0 +1,31 @@
+class Errors:
+    access_token_too_short = "The 'access_token' did not meet the expected length of 43 characters. Preventing "
+    "announcement query to OctoFarm"
+    access_token_not_saved = "Conditional error: 'access_token' was not saved properly. Please report a bug to the " \
+                             "plugin developers. Aborting "
+    base_url_not_provided = "The 'base_url' was not provided. Preventing announcement query to OctoFarm"
+    openid_config_unset = "Error connecting to OctoFarm. 'oidc_client_id' or 'oidc_client_secret' not set"
+    config_openid_missing = "Configuration error: 'oidc_client_id' or 'oidc_client_secret' not set"
+    ping_setting_unset = "'ping' config value not set. Aborting"
+
+class Keys:
+    persistence_uuid_key = "persistence_uuid"
+    device_uuid_key = "device_uuid"
+
+
+class Config:
+    access_token_length = 43
+    uuid_length = 36
+    persisted_data_file = "backup_excluded_data.json"
+    default_octofarm_host = "http://127.0.0.1"
+    default_octoprint_host = "http://127.0.0.1"
+    default_octofarm_port = 4000
+    default_ping_secs = 120
+
+
+class State:
+    BOOT = "boot"
+    SUCCESS = "success"
+    SLEEP = "sleep"
+    CRASHED = "crashed"
+    RETRY = "retry"