Review: Needs Fixing
generally ok, but there's several assert statements where you have change the
meaning, checking for Falsey/Truthy values rather than checking that the the
result is True, is False, is None, etc.
I've highlight a few with inline comments, but there are some others.
Diff comments:
>
> === modified file 'tests/functional/openlp_core/common/test_init.py'
> --- tests/functional/openlp_core/common/test_init.py 2017-11-18 23:14:28
> +0000
> +++ tests/functional/openlp_core/common/test_init.py 2017-12-09 16:45:47
> +0000
> @@ -259,7 +258,7 @@
> result = delete_file(None)
>
> # THEN: delete_file should return False
> - self.assertFalse(result, "delete_file should return False when
> called with None")
> + assert not result, "delete_file should return False when called with
> None"
wouldn't 'assert result is False' be better?
>
> def test_delete_file_path_success(self):
> """
> @@ -272,7 +271,7 @@
> result = delete_file(Path('path', 'file.ext'))
>
> # THEN: delete_file should return True
> - self.assertTrue(result, 'delete_file should return True when it
> successfully deletes a file')
> + assert result, 'delete_file should return True when it
> successfully deletes a file'
Same again here
>
> def test_delete_file_path_no_file_exists(self):
> """
> @@ -364,4 +363,4 @@
>
> # THEN: log.exception should be called and get_file_encoding
> should return None
> mocked_log.exception.assert_called_once_with('Error detecting
> file encoding')
> - self.assertIsNone(result)
> + assert not result
and here 'assert result is None'
--
https://code.launchpad.net/~trb143/openlp/asserts/+merge/335002
Your team OpenLP Core is subscribed to branch lp:openlp.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
