Add distribution detection in is_linux() function

- Optional argument to check Linux distro
- Add a test for the new argument
- Add some other tests to expand test coverage
- Remove Windows from testing
- If "distro" module is missing, create a replacement that returns False

.gitlab-ci.yml

@@ -63,19 +63,6 @@ test-macos:
   only:
     - master@openlp/openlp
 
-test-windows:
-  stage: test
-  tags:
-    - windows
-  script:
-    - C:\Users\raoul\GitLab-Runner\venv\Scripts\pytest.exe --color=no --disable-warnings --cov openlp
-    - mv .coverage windows.coverage
-  artifacts:
-    paths:
-      - windows.coverage
-  only:
-    - master@openlp/openlp
-
 test-display:
   stage: test
   image: openlp/angular
@@ -87,7 +74,7 @@ pages:
   stage: deploy
   image: openlp/debian
   script:
-    - python3-coverage combine linux.coverage macos.coverage windows.coverage
+    - python3-coverage combine linux.coverage macos.coverage
     - fixpaths .coverage
     - python3-coverage html
     - mv htmlcov public
@@ -99,6 +86,5 @@ pages:
   dependencies:
     - test-debian
     - test-macos
-    - test-windows
   only:
     - master@openlp/openlp

openlp/core/common/__init__.py

@@ -38,6 +38,13 @@ from PyQt5.QtCore import QCryptographicHash as QHash
 from PyQt5.QtNetwork import QAbstractSocket, QHostAddress, QNetworkInterface
 from chardet.universaldetector import UniversalDetector
 
+try:
+    from distro import id as distro_id
+except ImportError:
+    # The distro module is only valid for Linux, so if it doesn't exist, create a function that always returns False
+    def distro_id():
+        return False
+
 log = logging.getLogger(__name__ + '.__init__')
 
 
@@ -212,13 +219,17 @@ def is_macosx():
     return sys.platform.startswith('darwin')
 
 
-def is_linux():
+def is_linux(distro=None):
     """
     Returns true if running on a system with a linux kernel e.g. Ubuntu, Debian, etc
 
+    :param distro: If not None, check if running that Linux distro
     :return: True if system is running a linux kernel false otherwise
     """
-    return sys.platform.startswith('linux')
+    result = sys.platform.startswith('linux')
+    if result and distro:
+        result = result and distro == distro_id()
+    return result
 
 
 def is_64bit_instance():

scripts/check_dependencies.py

@@ -56,6 +56,7 @@ WIN32_MODULES = [
 LINUX_MODULES = [
     # Optical drive detection.
     'dbus',
+    'distro',
     'Xlib',
 ]
 

setup.py

@@ -101,6 +101,7 @@ using a computer and a data projector.""",
         'beautifulsoup4',
         'chardet',
         'dbus-python; platform_system=="Linux"',
+        'distro; platform_system=="Linux"',
         'lxml',
         'Mako',
         'pymediainfo >= 2.2',

tests/functional/openlp_core/common/test_common.py

@@ -22,11 +22,11 @@
 Functional tests to test the AppLocation class and related methods.
 """
 from pathlib import Path
-from unittest import TestCase
+from unittest import TestCase, skipUnless
 from unittest.mock import MagicMock, call, patch
 
 from openlp.core.common import Singleton, clean_button_text, de_hump, extension_loader, is_linux, is_macosx, is_win, \
-    normalize_str, path_to_module, trace_error_handler
+    is_64bit_instance, normalize_str, path_to_module, trace_error_handler
 
 
 class TestCommonFunctions(TestCase):
@@ -243,7 +243,7 @@ class TestCommonFunctions(TestCase):
         # GIVEN: Mocked out objects
         with patch('openlp.core.common.os') as mocked_os, patch('openlp.core.common.sys') as mocked_sys:
 
-            # WHEN: The mocked os.name and sys.platform are set to 'posix' and 'linux3' repectivly
+            # WHEN: The mocked os.name and sys.platform are set to 'posix' and 'linux3' repectively
             mocked_os.name = 'posix'
             mocked_sys.platform = 'linux3'
 
@@ -252,6 +252,40 @@ class TestCommonFunctions(TestCase):
             assert is_win() is False, 'is_win() should return False'
             assert is_macosx() is False, 'is_macosx() should return False'
 
+    @skipUnless(is_linux(), 'This can only run on Linux')
+    def test_is_linux_distro(self):
+        """
+        Test the is_linux() function for a particular Linux distribution
+        """
+        # GIVEN: Mocked out objects
+        with patch('openlp.core.common.os') as mocked_os, \
+                patch('openlp.core.common.sys') as mocked_sys, \
+                patch('openlp.core.common.distro_id') as mocked_distro_id:
+
+            # WHEN: The mocked os.name and sys.platform are set to 'posix' and 'linux3' repectively
+            #       and the distro is Fedora
+            mocked_os.name = 'posix'
+            mocked_sys.platform = 'linux3'
+            mocked_distro_id.return_value = 'fedora'
+
+            # THEN: The three platform functions should perform properly
+            assert is_linux(distro='fedora') is True, 'is_linux(distro="fedora") should return True'
+            assert is_win() is False, 'is_win() should return False'
+            assert is_macosx() is False, 'is_macosx() should return False'
+
+    def test_is_64bit_instance(self):
+        """
+        Test the is_64bit_instance() function
+        """
+        # GIVEN: Mocked out objects
+        with patch('openlp.core.common.sys') as mocked_sys:
+
+            # WHEN: The mocked sys.maxsize is set to 32-bit
+            mocked_sys.maxsize = 2**32
+
+            # THEN: The result should be False
+            assert is_64bit_instance() is False, 'is_64bit_instance() should return False'
+
     def test_normalize_str_leaves_newlines(self):
         # GIVEN: a string containing newlines
         string = 'something\nelse'

tests/openlp_core/common/test_network_interfaces.py

@@ -35,8 +35,9 @@ class FakeIP4InterfaceEntry(QObject):
     """
     Class to face an interface for testing purposes
     """
-    def __init__(self, name='lo'):
+    def __init__(self, name='lo', is_valid=True):
         self.my_name = name
+        self._is_valid = is_valid
         if name in ['localhost', 'lo']:
             self.my_ip = QNetworkAddressEntry()
             self.my_ip.setBroadcast(QHostAddress('255.0.0.0'))
@@ -75,7 +76,7 @@ class FakeIP4InterfaceEntry(QObject):
         return self.my_name
 
     def isValid(self):
-        return True
+        return self._is_valid
 
 
 class TestInterfaces(TestCase, TestMixin):
@@ -92,6 +93,7 @@ class TestInterfaces(TestCase, TestMixin):
             self.fake_lo = FakeIP4InterfaceEntry()
             self.fake_localhost = FakeIP4InterfaceEntry(name='localhost')
             self.fake_address = FakeIP4InterfaceEntry(name='eth25')
+            self.invalid_if = FakeIP4InterfaceEntry(name='invalid', is_valid=False)
 
     def tearDown(self):
         """
@@ -126,7 +128,7 @@ class TestInterfaces(TestCase, TestMixin):
         # GIVEN: Test environment
         call_debug = [
             call('Getting local IPv4 interface(es) information'),
-            call("Filtering out interfaces we don't care about: lo")
+            call('Filtering out interfaces we don\'t care about: lo')
         ]
 
         # WHEN: get_network_interfaces() is called
@@ -146,7 +148,7 @@ class TestInterfaces(TestCase, TestMixin):
         # GIVEN: Test environment
         call_debug = [
             call('Getting local IPv4 interface(es) information'),
-            call("Filtering out interfaces we don't care about: localhost")
+            call('Filtering out interfaces we don\'t care about: localhost')
         ]
 
         # WHEN: get_network_interfaces() is called
@@ -190,7 +192,7 @@ class TestInterfaces(TestCase, TestMixin):
         # GIVEN: Test environment
         call_debug = [
             call('Getting local IPv4 interface(es) information'),
-            call("Filtering out interfaces we don't care about: lo"),
+            call('Filtering out interfaces we don\'t care about: lo'),
             call('Checking for isValid and flags == IsUP | IsRunning'),
             call('Checking address(es) protocol'),
             call('Checking for protocol == IPv4Protocol'),
@@ -205,4 +207,26 @@ class TestInterfaces(TestCase, TestMixin):
 
         # THEN: There should be a fake 'eth25' interface
         mock_log.debug.assert_has_calls(call_debug)
-        assert interfaces == self.fake_address.fake_data, "There should have been only 'eth25' interface listed"
+        assert interfaces == self.fake_address.fake_data, 'There should have been only "eth25" interface listed'
+
+    @patch.object(openlp.core.common, 'log')
+    def test_network_interfaces_invalid(self, mock_log):
+        """
+        Test get_network_interfaces() returns an empty dictionary when there are no valid interfaces
+        """
+        # GIVEN: Test environment
+        call_debug = [
+            call('Getting local IPv4 interface(es) information'),
+            call('Checking for isValid and flags == IsUP | IsRunning')
+        ]
+        call_warning = [call('No active IPv4 network interfaces detected')]
+
+        # WHEN: get_network_interfaces() is called
+        with patch('openlp.core.common.QNetworkInterface') as mock_network_interface:
+            mock_network_interface.allInterfaces.return_value = [self.invalid_if]
+            interfaces = get_network_interfaces()
+
+        # THEN: There should be a fake 'eth25' interface
+        mock_log.debug.assert_has_calls(call_debug)
+        mock_log.warning.assert_has_calls(call_warning)
+        assert interfaces == {}, 'There should not be any interfaces listed'