Improve unit tests #191

tstenner · 2017-02-05T13:58:50Z

For an eventual py3-transition we need better unit test coverage.

What's planned / done:

Introduce a custom base class with better tempfile handling and asserts
Convert asserts / tempfile creation to use the class
Convert asserts from self.assert_ to more meaningful asserts (especially self.assert_(isinstance(...))
test for unicode / bytes explicitly

coveralls · 2017-02-05T14:48:10Z

Coverage remained the same at 59.188% when pulling f03c8b9 on tstenner:test_asserts into c2e3133 on bleachbit:master.

coveralls · 2017-02-05T14:49:54Z

Coverage remained the same at 59.188% when pulling f03c8b9 on tstenner:test_asserts into c2e3133 on bleachbit:master.

coveralls · 2017-02-05T15:02:41Z

Coverage remained the same at 59.188% when pulling 447a844 on tstenner:test_asserts into c2e3133 on bleachbit:master.

coveralls · 2017-02-05T15:18:36Z

Coverage decreased (-0.08%) to 59.111% when pulling cf59f94 on tstenner:test_asserts into c2e3133 on bleachbit:master.

coveralls · 2017-02-05T18:21:47Z

Changes Unknown when pulling 38922e7 on tstenner:test_asserts into ** on bleachbit:master**.

coveralls · 2017-02-05T19:07:49Z

Changes Unknown when pulling 7c6c6b9 on tstenner:test_asserts into ** on bleachbit:master**.

coveralls · 2017-02-05T20:25:05Z

Changes Unknown when pulling f6d77e8 on tstenner:test_asserts into ** on bleachbit:master**.

coveralls · 2017-02-06T19:13:04Z

Changes Unknown when pulling 77c3f78 on tstenner:test_asserts into ** on bleachbit:master**.

coveralls · 2017-02-06T20:14:54Z

Changes Unknown when pulling 1bc6410 on tstenner:test_asserts into ** on bleachbit:master**.

coveralls · 2017-02-07T17:28:22Z

Changes Unknown when pulling f4c46b6 on tstenner:test_asserts into ** on bleachbit:master**.

coveralls · 2017-02-07T17:31:17Z

Changes Unknown when pulling a0e2540 on tstenner:test_asserts into ** on bleachbit:master**.

coveralls · 2017-02-07T19:07:30Z

Changes Unknown when pulling a75a5ef on tstenner:test_asserts into ** on bleachbit:master**.

coveralls · 2017-02-07T19:13:11Z

Changes Unknown when pulling c925c41 on tstenner:test_asserts into ** on bleachbit:master**.

coveralls · 2017-02-07T19:23:07Z

Coverage decreased (-0.08%) to 59.111% when pulling 9ec8306 on tstenner:test_asserts into 391f08b on bleachbit:master.

az0 · 2017-02-08T03:36:33Z

tests/common.py



-class AssertFile:
+class BleachbitTestCase(unittest.TestCase):


It's a good idea to use inheritance here

az0 · 2017-02-08T03:40:20Z

tests/TestAction.py

-            cmd = 'cmd.exe /c dir'
-        if 'posix' == os.name:
-            cmd = 'dir'
+        cmds = {'nt': 'cmd.exe /c dir', 'posix': 'dir'}


This is a creative use of a dictionary. Nice.

az0 · 2017-02-08T03:46:16Z

tests/TestCommand.py

-        self.assert_(os.path.exists(path))
-        self.assert_(os.path.getsize(path) > 0)
+        self.assertExists(path)
+        self.assertGreater(os.path.getsize(path), 0)


assertGreater is another benefit of Python 2.7+

az0 · 2017-02-08T03:50:38Z

tests/TestFileUtilities.py

-        for test in tests_undo:
-            self.assertEqual(extended_path_undo(test[0]), test[1])
+        for short, extended in tests:
+            # already extended path shouldn't be changed


These are logical tests

coveralls · 2017-02-08T03:56:24Z

Coverage remained the same at 59.188% when pulling ed370d5 on tstenner:test_asserts into 391f08b on bleachbit:master.

az0 · 2017-02-08T03:58:22Z

This is great work. I love a cleanup like this that reduces code.

I could merge this now, but would you rather wait? I see only 2/4 items are checked off.

ROCKNROLLKID · 2017-02-08T05:45:31Z

I am seeing merge conflicts with this. Will need to fix that first.

tstenner · 2017-02-08T10:17:47Z

I'd rather go through the last remaining files first and then rebase the PR, after that I'd also be in favor of merging it (less conflicts for me and everyone else). I can always open another PR for the remaining items.

ROCKNROLLKID · 2017-02-08T19:09:58Z

Are we holding back Python 3 coding for the next update then?

az0 · 2017-02-09T03:20:53Z

@tstenner OK, please write a note when it's ready.

@ROCKNROLLKID I would like to hold major changes including Python 3 and GTK 3 until after BleachBit 2.0 is released. Then it may also take a follow-up maintenance release to fix important regressions. After that I would like to evaluate how well Python 3 and GTK 3 work, especially on Windows. Given the recent work to shrink the new GTK 2 on Windows for BleachBit 2.0, I assume that I can also shrink Python 3 and GTK 3. This pull request is a stepping stone for Python 3 because it improves test coverage.

coveralls · 2017-02-09T15:46:08Z

Coverage remained the same at 59.058% when pulling 77d91c8 on tstenner:test_asserts into 83d09cc on bleachbit:master.

coveralls · 2017-02-09T16:49:35Z

Coverage remained the same at 59.058% when pulling 772a35f on tstenner:test_asserts into 83d09cc on bleachbit:master.

coveralls · 2017-02-09T17:03:37Z

Coverage remained the same at 59.058% when pulling 06f95f4 on tstenner:test_asserts into 83d09cc on bleachbit:master.

coveralls · 2017-02-10T07:37:48Z

Coverage decreased (-0.08%) to 58.917% when pulling 0aa45ad on tstenner:test_asserts into 66271f4 on bleachbit:master.

coveralls · 2017-02-10T08:05:46Z

Coverage remained the same at 58.993% when pulling 70674c2 on tstenner:test_asserts into 66271f4 on bleachbit:master.

coveralls · 2017-02-10T08:12:11Z

Coverage remained the same at 58.993% when pulling 37d3e12 on tstenner:test_asserts into 66271f4 on bleachbit:master.

tstenner · 2017-02-10T08:17:36Z

The test for locked files fails because the worker reports that it cleaned too many bytes (6 instead of 3), but I (think I) didn't change anything relevant in the failing test. Could someone with a windows installation take a look at it?

az0 · 2017-02-11T03:13:40Z

The error 6 != 3 looks like something I broke related to the new WindowsWipe code. I will work on it.

az0 · 2017-02-11T18:07:14Z

The 6 != 3 error was fixed in commit 193e07a

tstenner · 2017-02-12T12:17:27Z

The unit tests pass now, travis hits a time limit somehow (downloading the appveyor artifacts?). We should merge this, I'll tackle the rest of the todo-list in a separate PR

az0 · 2017-02-13T03:59:50Z

Yes, Travis hit a time limit waiting on AppVeyor, while Appveyor failed because of the errno typo.

tstenner force-pushed the test_asserts branch 3 times, most recently from 7e5af79 to f03c8b9 Compare February 5, 2017 14:46

tstenner force-pushed the test_asserts branch from f03c8b9 to 447a844 Compare February 5, 2017 14:59

tstenner force-pushed the test_asserts branch from cf59f94 to 38922e7 Compare February 5, 2017 18:18

tstenner force-pushed the test_asserts branch from 38922e7 to 7c6c6b9 Compare February 5, 2017 19:04

tstenner force-pushed the test_asserts branch from c925c41 to 9ec8306 Compare February 7, 2017 19:19

tstenner force-pushed the test_asserts branch from 9ec8306 to ed370d5 Compare February 7, 2017 19:46

az0 reviewed Feb 8, 2017

View reviewed changes

tests/common.py

class AssertFile:

class BleachbitTestCase(unittest.TestCase):

Copy link

Member

az0 Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good idea to use inheritance here

az0 reviewed Feb 8, 2017

View reviewed changes

tstenner force-pushed the test_asserts branch from ed370d5 to 77d91c8 Compare February 9, 2017 15:42

tstenner force-pushed the test_asserts branch from 4c4956f to 06f95f4 Compare February 9, 2017 17:00

tstenner force-pushed the test_asserts branch 2 times, most recently from 2a01aef to 0aa45ad Compare February 10, 2017 07:34

tstenner force-pushed the test_asserts branch from 37d3e12 to 1369d9c Compare February 10, 2017 08:16

tstenner added 3 commits February 12, 2017 07:08

Use custom TestCase-class

ebefcbb

Convert assert_s

77726c3

temp file handling

fc4ac0a

tstenner force-pushed the test_asserts branch 2 times, most recently from 7e8380a to 984ee27 Compare February 12, 2017 07:21

Fix typo

d0be80a

tstenner force-pushed the test_asserts branch from 984ee27 to d0be80a Compare February 12, 2017 08:18

az0 merged commit c164a0d into bleachbit:master Feb 13, 2017

tstenner deleted the test_asserts branch February 13, 2017 09:25



		class AssertFile:
		class BleachbitTestCase(unittest.TestCase):

Improve unit tests #191

Improve unit tests #191

Conversation

tstenner commented Feb 5, 2017 • edited

coveralls commented Feb 5, 2017

coveralls commented Feb 5, 2017

coveralls commented Feb 5, 2017

coveralls commented Feb 5, 2017

coveralls commented Feb 5, 2017

coveralls commented Feb 5, 2017

coveralls commented Feb 5, 2017

coveralls commented Feb 6, 2017

coveralls commented Feb 6, 2017

coveralls commented Feb 7, 2017

coveralls commented Feb 7, 2017

coveralls commented Feb 7, 2017

coveralls commented Feb 7, 2017

coveralls commented Feb 7, 2017

az0 Feb 8, 2017

Choose a reason for hiding this comment

az0 Feb 8, 2017

Choose a reason for hiding this comment

az0 Feb 8, 2017

Choose a reason for hiding this comment

az0 Feb 8, 2017

Choose a reason for hiding this comment

coveralls commented Feb 8, 2017

az0 commented Feb 8, 2017

ROCKNROLLKID commented Feb 8, 2017

tstenner commented Feb 8, 2017

ROCKNROLLKID commented Feb 8, 2017

az0 commented Feb 9, 2017

coveralls commented Feb 9, 2017

coveralls commented Feb 9, 2017

coveralls commented Feb 9, 2017

coveralls commented Feb 10, 2017

coveralls commented Feb 10, 2017

coveralls commented Feb 10, 2017

tstenner commented Feb 10, 2017

az0 commented Feb 11, 2017

az0 commented Feb 11, 2017

tstenner commented Feb 12, 2017

az0 commented Feb 13, 2017

tstenner commented Feb 5, 2017 •

edited