ci: unittests on PR and Release workflows (#85)

.github/workflows/pr.yml

@@ -35,6 +35,16 @@ jobs:
     with:
       python_version: ${{ needs.pyversion.outputs.pyversion }}
 
+  unit_tests:
+    name: Unit Tests
+    needs: pyversion
+    permissions:
+      contents: read
+      pull-requests: write
+    uses: ./.github/workflows/sub_unittest.yml
+    with:
+      python_version: ${{ needs.pyversion.outputs.pyversion }}
+
   documentation_check:
     name: API documentation
     needs: pyversion
@@ -57,6 +67,7 @@ jobs:
     name: Fetch the updated documentation
     needs:
       - code_format
+      - unit_tests
       - documentation_check
       - docker_image_test_build
     runs-on: ubuntu-latest

.github/workflows/release.yml

@@ -57,6 +57,13 @@ jobs:
     with:
       python_version: ${{ needs.pyversion.outputs.pyversion }}
 
+  unit_tests:
+    name: Validate all Unit Tests pass
+    needs: pyversion
+    uses: ./.github/workflows/sub_unittest.yml
+    with:
+      python_version: ${{ needs.pyversion.outputs.pyversion }}
+
   documentation_check:
     name: Validate that the API documentation is up to date
     needs: pyversion
@@ -71,6 +78,7 @@ jobs:
     needs:
       - rc
       - code_format
+      - unit_tests
       - documentation_check
       - pyversion
     concurrency: release

.github/workflows/sub_unittest.yml

@@ -0,0 +1,49 @@
+name: (sub) Unit Tests
+
+defaults:
+  run:
+    shell: bash
+
+on:
+  workflow_call:
+    inputs:
+      python_version:
+        description: A version of Python to install
+        type: string
+        required: true
+
+jobs:
+
+  unittests:
+    name: Verify unit tests are successful
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v3
+
+      - name: Install Python
+        uses: actions/setup-python@v4
+        with:
+          python-version: ${{ inputs.python_version }}
+
+      - name: Install Poetry
+        uses: Gr1N/setup-poetry@v8
+
+      - name: Create Poetry venv
+        run: |
+          poetry env use ${{ inputs.python_version }}
+          poetry install
+
+      - name: Run unit tests and coverage
+        run: poetry run make test_coverage
+
+      # requires pull-requests: write permissions when triggered from PRs
+      - name: Get coverage
+        uses: orgoro/coverage@v3.1
+        with:
+            coverageFile: coverage.xml
+            token: ${{ secrets.GITHUB_TOKEN }}
+            thresholdAll: 0.95
+            thresholdNew: 0.90
+            thresholdModified: 0.95
+        if: ${{ github.event_name == 'pull_request' }}

Makefile

@@ -1,20 +1,23 @@
 lint:
-	flake8 panos_upgrade_assurance
+	flake8 panos_upgrade_assurance tests
 
 security:
 	bandit -c pyproject.toml -r .
 
 format_check:
-	black --diff --check panos_upgrade_assurance
+	black --diff --check panos_upgrade_assurance tests
 
 format:
-	black panos_upgrade_assurance
+	black panos_upgrade_assurance tests
+
+test_coverage:
+	pytest --cov panos_upgrade_assurance --cov-report=term-missing --cov-report=xml:coverage.xml
 
 documentation:
 	pydoc-markdown
 
 check_line_length:
-	@for FILE in $$(ls panos_upgrade_assurance/[a-z]*.py); do \
+	@for FILE in $$(find . -type f -name '*.py'); do \
 		echo $$FILE; \
 		LN=0; \
 		while read -r line; do \
@@ -26,4 +29,4 @@ check_line_length:
 		done < "$$FILE"; \
 	done
 
-all: lint format security documentation
+all: lint format security test_coverage documentation

tests/test_check_firewall.py

@@ -934,7 +934,7 @@ def test_run_readiness_checks_wrong_data_type_exception(self, check_firewall_moc
         checks_configuration = ["check1", [123]]
         report_style = False
 
-        with pytest.raises(WrongDataTypeException) as exception_msg:
+        with pytest.raises(WrongDataTypeException):
             check_firewall_mock.run_readiness_checks(checks_configuration, report_style)
 
         # raise exceptions.WrongDataTypeException(f"Wrong configuration format for check: {check}.")
@@ -964,7 +964,7 @@ def test_run_snapshots(self, check_firewall_mock):
     def test_run_snapshots_wrong_data_type_exception(self, check_firewall_mock):
         snapshots_config = ["snapshot1", 123]
 
-        with pytest.raises(WrongDataTypeException) as exception_msg:
+        with pytest.raises(WrongDataTypeException):
             check_firewall_mock.run_snapshots(snapshots_config)
 
         # raise exceptions.WrongDataTypeException(f"Wrong configuration format for snapshot: {snap_type}.")

tests/test_snapshot_compare.py

@@ -1,5 +1,6 @@
 import pytest
 from unittest.mock import MagicMock
+from deepdiff import DeepDiff
 from panos_upgrade_assurance.snapshot_compare import SnapshotCompare
 from panos_upgrade_assurance.exceptions import WrongDataTypeException, MissingKeyException, SnapshotSchemeMismatchException
 from snapshots import snap1, snap2
@@ -154,7 +155,11 @@ def test_calculate_diff_on_dicts_empty_dicts(self):
 
         result = SnapshotCompare.calculate_diff_on_dicts(left_snapshot, right_snapshot)
 
-        assert result == {}
+        assert result == {
+            "missing": {"passed": True, "missing_keys": []},
+            "added": {"passed": True, "added_keys": []},
+            "changed": {"passed": True, "changed_raw": {}},
+        }
 
     # TODO test_calculate_diff_on_dicts properties flag
 
@@ -257,6 +262,21 @@ def test_get_diff_and_threshold_invalid_count_change_threshold(self, count_chang
         with pytest.raises(WrongDataTypeException, match="The threshold should be a percentage value between 0 and 100."):
             snapshot_compare.get_diff_and_threshold(report_type="nics", count_change_threshold=count_change_threshold)
 
+    @pytest.mark.parametrize(
+        "left_snapshot, right_snapshot, expected_change_pct",
+        [
+            ({"nics": {}}, {"nics": {}}, 0),
+            ({"nics": {}}, {"nics": {"ethernet1/2": "up", "ethernet1/3": "down", "tunnel": "up"}}, 100),
+            ({"nics": {"ethernet1/2": "up", "ethernet1/3": "up", "tunnel": "up"}}, {"nics": {}}, 100),
+        ],
+    )
+    def test_get_diff_and_threshold_empty_dicts_count_change(self, left_snapshot, right_snapshot, expected_change_pct):
+        change_threshold = 40
+        snapshot_compare = SnapshotCompare(left_snapshot, right_snapshot)
+        result = snapshot_compare.get_diff_and_threshold(report_type="nics", count_change_threshold=change_threshold)
+
+        assert result["count_change_percentage"]["change_percentage"] == expected_change_pct
+
     def test_get_count_change_percentage_no_thresholds(self):
         snapshot_compare = SnapshotCompare(snap1, snap2)
         assert snapshot_compare.get_count_change_percentage(report_type="session_stats") is None
@@ -394,7 +414,17 @@ def test_get_count_change_percentage(self, thresholds, expected_result):
                     }
                 },
             ),
-            # ( ['nics', 'arp_table'], {} ),  # TODO add test for non common keys - e.g. arp_table
+            (
+                ["arp_table"],
+                {
+                    "arp_table": {
+                        "added": {"added_keys": ["ethernet1/1_10.0.2.11"], "passed": False},
+                        "changed": {"changed_raw": {}, "passed": True},
+                        "missing": {"missing_keys": ["ethernet1/2_10.0.1.1", "ethernet1/1_10.0.2.1"], "passed": False},
+                        "passed": False,
+                    }
+                },
+            ),
             (
                 [
                     {
@@ -420,7 +450,9 @@ def test_compare_snapshots(self, reports, expected_result):
         snapshot_compare = SnapshotCompare(snap1, snap2)
         result = snapshot_compare.compare_snapshots(reports)
 
-        assert result == expected_result
+        assert not DeepDiff(
+            result, expected_result, ignore_order=True
+        )  # assert == doesnt work for nested objects and unordered lists
 
     # NOTE reports are already validated in ConfigParser called from the compare_snapshots method
     # so below check is never executed