requested changes

Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>

Author: OmkarSarkar204

.github/workflows/pytest.yml

@@ -1,19 +1,17 @@
 name: Pytest
 
-# https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python#packaging-workflow-data-as-artifacts
-
 on:
   pull_request:
     paths:
-      - '**/*.py'        # Watch for changes in any Python files
-      - 'pyproject.toml'  # Watch for changes in the pyproject.toml file
+      - '**/*.py'
+      - 'pyproject.toml'
       - '.github/workflows/pytest.yml'
   push:
     branches:
-      - master           # Only run on push to master branch
+      - master
     paths:
-      - '**/*.py'        # Watch for changes in any Python files
-      - 'pyproject.toml'  # Watch for changes in the pyproject.toml file
+      - '**/*.py'
+      - 'pyproject.toml'
       - '.github/workflows/pytest.yml'
   workflow_dispatch:
   release:
@@ -29,28 +27,32 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        # os: [ubuntu-latest, macos-latest, windows-latest]
-        # python-version: ["3.9", "3.10", "3.11", "3.12", "pypy3.9", "pypy3.10"]
-        os: [ubuntu-latest]
-        python-version: ["3.9", "3.14", "3.14t"]
+        include:
+          - os: ubuntu-latest
+            python-version: "3.9"  # to make sure the minimum AMC supported python version does not break
+          - os: ubuntu-latest
+            python-version: "3.14"  # to make sure the latest AMC supported python version does not break
+          - os: macos-latest
+            python-version: "3.14"  # The macOS .dmg file uses the latest python version
 
     steps:
       - name: Harden the runner (Audit all outbound calls)
+        if: matrix.os != 'windows-latest'
         uses: step-security/harden-runner@5ef0c079ce82195b2a36a210272d6b661572d83e # v2.14.2
         with:
           egress-policy: audit
 
       - name: Checkout
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
 
-      # https://docs.astral.sh/uv/guides/integration/github/
       - name: Install uv and set the Python version
         uses: astral-sh/setup-uv@eac588ad8def6316056a12d4907a9d4d84ff7a3b # v7.3.0
         with:
           python-version: ${{ matrix.python-version }}
           activate-environment: true
 
-      - name: Install system dependencies for GUI testing
+      - name: Install system dependencies for GUI testing on linux
+        if: matrix.os == 'ubuntu-latest'
         run: |
           sudo apt-get update
           # Only install system Tcl/Tk for Python < 3.13 (newer versions bundle their own)
@@ -62,35 +64,32 @@ jobs:
             # Only install tools, not Tcl/Tk libraries
             sudo apt-get install -y scrot xdotool x11-utils gnome-screenshot
           fi
+          sudo apt-get install -y xvfb libx11-6 libxrender1 libxext6 libsm6
           python3 --version && python3 -c "import tkinter; print(tkinter.TclVersion, tkinter.TkVersion)"
 
-      - name: Ensure Tcl/Tk search paths
+      - name: Install system dependencies for GUI testing on macOS
+        if: matrix.os == 'macos-latest'
         run: |
-          # Only set Tcl/Tk paths for Python < 3.13
-          if [[ "${{ matrix.python-version }}" < "3.13" ]]; then
-            echo "Setting Tcl/Tk paths for Python ${{ matrix.python-version }}"
-            echo "TCL_LIBRARY=/usr/share/tcltk/tcl8.6" >> $GITHUB_ENV
-            echo "TK_LIBRARY=/usr/share/tcltk/tk8.6" >> $GITHUB_ENV
-          else
-            echo "Python ${{ matrix.python-version }} uses bundled Tcl/Tk, no custom paths needed"
-          fi
+          brew install tcl-tk
+          echo "LDFLAGS=-L$(brew --prefix tcl-tk)/lib" >> $GITHUB_ENV
+          echo "CPPFLAGS=-I$(brew --prefix tcl-tk)/include" >> $GITHUB_ENV
+          echo "PKG_CONFIG_PATH=$(brew --prefix tcl-tk)/lib/pkgconfig" >> $GITHUB_ENV
+          python3 -c "import tkinter; print('Tk OK:', tkinter.TclVersion, tkinter.TkVersion)"
 
       - name: Install dependencies and application
-        # without --editable, the coverage report is not generated correctly
         run: |
           uv pip install --editable .[dev,ci_headless_tests]
 
       - name: Download ArduCopter SITL (if available)
+        if: matrix.os == 'ubuntu-latest'
         run: |
-          # Create cache key based on current quarter (YYYY-Q)
           CURRENT_YEAR=$(date +%Y)
           CURRENT_MONTH=$(date +%m)
           QUARTER=$(( (CURRENT_MONTH-1)/3 + 1 ))
           CACHE_KEY="${CURRENT_YEAR}-Q${QUARTER}"
 
           echo "Cache key: ${CACHE_KEY}"
 
-          # Check if we have cached SITL files for this quarter
           if [ -d "sitl-cache/${CACHE_KEY}" ] && [ -f "sitl-cache/${CACHE_KEY}/arducopter" ]; then
             echo "Using cached SITL files from ${CACHE_KEY}"
             mkdir -p sitl/
@@ -99,20 +98,16 @@ jobs:
             echo "Downloading fresh SITL files for ${CACHE_KEY}"
             mkdir -p sitl/ sitl-cache/${CACHE_KEY}/
 
-            # Download latest ArduCopter SITL from official firmware server
             curl -L -o sitl/arducopter https://firmware.ardupilot.org/Copter/latest/SITL_x86_64_linux_gnu/arducopter
             curl -L -o sitl/firmware-version.txt https://firmware.ardupilot.org/Copter/latest/SITL_x86_64_linux_gnu/firmware-version.txt
             curl -L -o sitl/git-version.txt https://firmware.ardupilot.org/Copter/latest/SITL_x86_64_linux_gnu/git-version.txt
 
-            # Cache the downloaded files
             cp sitl/* sitl-cache/${CACHE_KEY}/
           fi
 
-          # Make executable and verify
           chmod +x sitl/arducopter
           ls -la sitl/
 
-          # Set environment variables
           echo "SITL_BINARY=$(pwd)/sitl/arducopter" >> $GITHUB_ENV
           echo "SITL_AVAILABLE=true" >> $GITHUB_ENV
           echo "SITL version: $(cat sitl/git-version.txt)"
@@ -132,19 +127,25 @@ jobs:
         continue-on-error: false
         run: |
           export LIBGL_ALWAYS_SOFTWARE=1
-          export DISPLAY=:99
-          # disable X authentication
-          export XAUTHORITY=/dev/null
-          # disable access control restrictions
-          Xvfb :99 -screen 0 1024x768x16 -ac &
-          # ensure Xvfb is fully started before running tests
-          sleep 2
-          if [ "$SITL_AVAILABLE" = "true" ]; then
-            echo "Running tests with SITL support"
-            uv run pytest --cov=ardupilot_methodic_configurator --cov-report=xml:tests/coverage.xml --md=tests/results-${{ matrix.python-version }}.md --junit-xml=tests/results-junit.xml -m "sitl or not sitl"
+
+          if [[ "${{ matrix.os }}" == "ubuntu-latest" ]]; then
+            export DISPLAY=:99
+            export XAUTHORITY=/dev/null
+            Xvfb :99 -screen 0 1024x768x16 -ac &
+            sleep 2
+
+            if [ "$SITL_AVAILABLE" = "true" ]; then
+              FINAL_MARKER="sitl or not sitl"
+            else
+              FINAL_MARKER="not sitl"
+            fi
+
+            echo "Running Ubuntu tests with markers: $FINAL_MARKER"
+            uv run pytest --cov=ardupilot_methodic_configurator --cov-report=xml:tests/coverage.xml --md=tests/results-${{ matrix.python-version }}.md --junit-xml=tests/results-junit.xml -m "$FINAL_MARKER"
+
           else
-            echo "Running tests without SITL (mocked tests only)"
-            uv run pytest --cov=ardupilot_methodic_configurator --cov-report=xml:tests/coverage.xml --md=tests/results-${{ matrix.python-version }}.md --junit-xml=tests/results-junit.xml -m "not sitl"
+            echo "Running macOS headless tests (skipping GUI files entirely)"
+            uv run pytest --cov=ardupilot_methodic_configurator --cov-report=xml:tests/coverage.xml --md=tests/results-${{ matrix.python-version }}.md --junit-xml=tests/results-junit.xml -m "not gui" --ignore-glob="tests/acceptance_*.py" --ignore-glob="tests/*frontend*.py" --ignore-glob="tests/*tkinter*.py"
           fi
 
       - name: Fix coverage paths
@@ -156,27 +157,24 @@ jobs:
 
       - name: Display test results as GitHub job summary
         run: cat tests/results-${{ matrix.python-version }}.md >> $GITHUB_STEP_SUMMARY
-        # Use always() to always run this step to publish test results when there are test failures
         if: ${{ always() }}
 
       - name: Upload coverage xml report
+        if: matrix.os == 'ubuntu-latest' && always()
         uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0
         with:
             name: coverage-${{ matrix.python-version }}-xml
             path: tests/*.xml
             retention-days: 1
-        # Use always() to always run this step to publish test results when there are test failures
-        if: ${{ always() }}
 
       - name: Upload coverage report
+        if: matrix.os == 'ubuntu-latest' && always()
         uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0
         with:
             name: coverage-${{ matrix.python-version }}
             path: .coverage
             include-hidden-files: true
             retention-days: 1
-        # Use always() to always run this step to publish test results when there are test failures
-        if: ${{ always() }}
 
   upload_coverage_to_coveralls:
     if: (github.event_name == 'push' && github.ref == 'refs/heads/master') && (success() || failure())
@@ -203,12 +201,10 @@ jobs:
           github-token: ${{ secrets.GITHUB_TOKEN }}
           files: coverage.xml
 
-      # TODO: create a badge that presents the result of the Upload coverage xml report step
-
   check_coverage:
     if: success() || failure()
     runs-on: ubuntu-latest
-    needs: pytest  # This will ensure this job runs after 'pytest'
+    needs: pytest
 
     steps:
       - name: Harden the runner (Audit all outbound calls)
@@ -224,11 +220,10 @@ jobs:
         with:
           name: coverage-3.9
 
-      # https://docs.astral.sh/uv/guides/integration/github/
       - name: Install uv and set the Python version
         uses: astral-sh/setup-uv@eac588ad8def6316056a12d4907a9d4d84ff7a3b # v7.3.0
         with:
-          python-version: '3.9'  # Match with the coverage report Python version
+          python-version: '3.9'
           activate-environment: true
 
       - name: Install dependencies
@@ -237,7 +232,6 @@ jobs:
 
       - name: Check coverage
         run: |
-          # Check if pytest job failed
           if [ "${{ needs.pytest.result }}" == "failure" ]; then
             echo "Pytest failed - failing coverage check"
             exit 1
@@ -248,11 +242,9 @@ jobs:
     if: always()
     name: "Publish Tests Results"
     runs-on: ubuntu-latest
-    needs: pytest  # This will ensure this job runs after 'pytest'
+    needs: pytest
     permissions:
       checks: write
-
-      # only needed unless run with comment_mode: off
       pull-requests: write
 
     steps:
@@ -296,7 +288,6 @@ jobs:
           path: badge.svg
 
       - name: Upload badge to Gist
-        # Upload only for master branch
         if: >
           github.event_name == 'workflow_run' && github.event.workflow_run.head_branch == 'master' ||
           github.event_name != 'workflow_run' && github.ref == 'refs/heads/master'
@@ -309,7 +300,7 @@ jobs:
   add_coverage_to_pullrequest:
     if: github.event_name == 'pull_request' && (success() || failure())
     runs-on: ubuntu-latest
-    needs: pytest  # This will ensure this job runs after 'pytest'
+    needs: pytest
     permissions:
       contents: read
       pull-requests: write

pytest.ini

@@ -14,3 +14,4 @@ markers =
     integration: mark a test as an integration test
     slow: mark a test as slow running
     sitl: mark a test as requiring SITL (real ArduPilot simulation)
+    gui: requires real GUI (tkinter, windows, cocoa)

tests/acceptance_battery_monitor.py

@@ -30,6 +30,8 @@
 )
 from ardupilot_methodic_configurator.plugin_factory import plugin_factory
 
+pytestmark = pytest.mark.gui
+
 # pylint: disable=redefined-outer-name,protected-access,too-many-lines
 
 

tests/conftest.py

@@ -498,3 +498,11 @@ def sitl_flight_controller(sitl_manager: SITLManager) -> Generator[FlightControl
 
     # Cleanup connection but keep SITL running for subsequent tests
     fc.disconnect()
+
+
+@pytest.fixture(autouse=True)
+def mock_center_window_for_macos_only() -> None:
+    """Only mock center_window on macOS to prevent Tkinter crashes. Let Linux run normally."""
+    if platform.system() == "Darwin":
+        patcher = patch("ardupilot_methodic_configurator.frontend_tkinter_base_window.BaseWindow.center_window")
+        patcher.start()

tests/gui_frontend_tkinter_parameter_editor.py

@@ -106,6 +106,7 @@ def test_show_about_window(self, mocker) -> None:  # pylint: disable=too-many-lo
             # Check window properties
             assert about_window.title() == "About"
             # Check that geometry contains the expected size (position may vary)
+            about_window.update_idletasks()
             geometry = about_window.geometry()
             assert "650x340" in geometry
 

tests/test_backend_mavftp.py

@@ -17,8 +17,7 @@
 
 # from unittest.mock import patch
 from io import StringIO
-
-from pymavlink import mavutil
+from unittest.mock import MagicMock
 
 # from ardupilot_methodic_configurator.backend_mavftp import ERR_NoErrorCodeInPayload
 # from ardupilot_methodic_configurator.backend_mavftp import ERR_NoErrorCodeInNack
@@ -65,10 +64,10 @@ def setUp(self) -> None:
         logger.setLevel(logging.DEBUG)
 
         # Mock mavutil.mavlink_connection to simulate a connection
-        self.mock_master = mavutil.mavlink_connection(device="udp:localhost:14550", source_system=1)
+        self.mock_master = MagicMock()
 
         # Initialize MAVFTP instance for testing
-        self.mav_ftp = MAVFTP(self.mock_master, target_system=1, target_component=1)
+        self.mav_ftp = MAVFTP(self.mock_master, 1, 0)
 
     def tearDown(self) -> None:
         self.log_stream.seek(0)

tests/test_frontend_tkinter_rich_text.py

@@ -184,8 +184,14 @@ def tearDown(self) -> None:
     def test_get_widget_font_family_and_size(self) -> None:
         label = ttk.Label(self.root, text="Test")
         family, size = get_widget_font_family_and_size(label)
-        expected_family = ["Segoe UI"] if platform_system() == "Windows" else ["Helvetica", "sans-serif"]
-        expected_size = [9] if platform_system() == "Windows" else [-12, 10]
+        expected_family = (
+            ["Segoe UI"]
+            if platform_system() == "Windows"
+            else [".AppleSystemUIFont"]
+            if platform_system() == "Darwin"
+            else ["Helvetica", "sans-serif"]
+        )
+        expected_size = [9] if platform_system() == "Windows" else [13] if platform_system() == "Darwin" else [-12, 10]
         assert isinstance(family, str)
         assert isinstance(size, int)
         assert family in expected_family