openlp-core team mailing list archive

[Simon Hanna <simon@xxxxxxxxxxx>]

Simon Hanna has proposed merging lp:~thelinuxguy/openlp/improve-logging into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)

For more details, see:
https://code.launchpad.net/~thelinuxguy/openlp/improve-logging/+merge/352939

Instead of raising an error when converting paths, return None and log an error.

The method is allowed to return None, so calling functions should handle that. Raising an error is very rude behavior and the output just said that the input was invalid with no real clue as to what is wrong.

Now None is returned without causing too much hassle, and a proper error is logged mentioning what exactly caused the error.

The test case was updated.

This is sort of related to https://bugs.launchpad.net/openlp/+bug/1786601 in that this would have been nice to actually know what went wrong and failback to None.
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~thelinuxguy/openlp/improve-logging into lp:openlp.

=== modified file 'openlp/core/common/path.py'
--- openlp/core/common/path.py	2018-01-13 04:57:16 +0000
+++ openlp/core/common/path.py	2018-08-12 11:29:05 +0000
@@ -206,7 +206,8 @@
     :rtype: openlp.core.common.path.Path | None
     """
     if not isinstance(string, str):
-        raise TypeError('parameter \'string\' must be of type str')
+        log.error('parameter \'string\' must be of type str, got {} which is a {} instead'.format(string, type(string)))
+        return None
     if string == '':
         return None
     return Path(string)

=== modified file 'tests/functional/openlp_core/common/test_path.py'
--- tests/functional/openlp_core/common/test_path.py	2018-01-13 04:57:16 +0000
+++ tests/functional/openlp_core/common/test_path.py	2018-08-12 11:29:05 +0000
@@ -271,13 +271,12 @@
 
     def test_str_to_path_type_error(self):
         """
-        Test that `str_to_path` raises a type error when called with an invalid type
+        Test that `str_to_path` returns None if called with invalid information
         """
         # GIVEN: The `str_to_path` function
         # WHEN: Calling `str_to_path` with an invalid Type
-        # THEN: A TypeError should have been raised
-        with self.assertRaises(TypeError):
-            str_to_path(Path())
+        # THEN: None is returned
+        assert str_to_path(Path()) is None
 
     def test_str_to_path_empty_str(self):
         """