Code review comment for lp://staging/~senan/ubuntu-autopilot-tests/DiskUsageAnalyser

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

I'm still seeing a lot of pep8 errors:

pep8 DiskUsageAnalyser/*.py

Can these be fixed?

Also, can you rename DiskUsageAnalyser to diskusageanalyser or disk_usage_analyser to follow along with the all lowercase names. It's helpful. I would rename the .py file as well to all lowercase or use _.

Finally, the tests fail on my local machine. I'll attach the log: http://paste.ubuntu.com/7034425/

I would suggest a few other modifications:

DiskUsageAnalyser.test_diskUsageAnalyser.DiskUsageAnalyzerTests.test_scan_remote_folder -- what are we trying to test here? We type in text, then hit cancel.

The asserts like self.assertThat(path_bar_home_label.label,Equals(key_input.lstrip('/'))) fail for me when I run from somewhere other than /home.

This is another example where /home should be mocked, and we can mock files and folders as part of it to more thoroughly test things. Have a look at http://www.voidspace.org.uk/python/mock/ and for an example see http://bazaar.launchpad.net/~music-app-dev/music-app/trunk/view/head:/tests/autopilot/music_app/tests/__init__.py, specifically _patch_home.

review: Needs Fixing

« Back to merge proposal