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 : openlp-core@lists.launchpad.net Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp