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

Senan hey,

So what i was trying to say in IRC is that you are repeating code in a few of your tests. So lets use the test_scan_local_folder for this example and you will be able to use it in the other test just as easy

lets start with this part of the test

def test_scan_local_folder(self):
    self.gear_menu_option_button = self.app.select_single(BuilderName='menu-button')
    self.pointing_device.move_to_object(self.gear_menu_option_button)
    self.pointing_device.click()
    self.create_scan_folder_menu_item = self.app.select_single('GtkModelMenuItem',action_name= 'win.scan-folder')
    self.pointing_device.move_to_object(self.create_scan_folder_menu_item)
    self.pointing_device.click()

you use this same process in two tests but selecting a different GtkModelMenuItem for each test, so this can be broken out into its own function and you pass in the name of the GtkModelMenuItem you need for each test

you should use more descriptive names than this, i'm just showing examples

def test_scan_local_folder(self):
    self.open_folder_dialog('win.scan-folder')
    # .. Rest of test ..

def open_folder_dialog(self, item_name):
    gear_menu_option_button = self.app.select_single(BuilderName='menu-button')
    self.pointing_device.click_object(self.gear_menu_option_button)
    #now select the passed in menu item
    create_scan_folder_menu_item = self.app.select_single('GtkModelMenuItem',action_name=item_name)
    self.pointing_device.click_object(create_scan_folder_menu_item)

so now for your tests you can reuse this and just pass in the action_name of the menu item
i.e
self.open_folder_dialog('win.scan-folder')
self.open_folder_dialog('win.scan-remote')

Thats an example of what i mean by breaking up the tests a bit and re-using code makes it easier to maintain.

Regarding my comments on finishing the tests inside the test function, currently on some of the tests you call self.disk_usage_display_common() as the last call of your test, this in effect takes us out of the test function and we have to go looking elswhere to see what the intent of the test function is trying to assert. Each test method should work like

def Test
    1) setup the test (this is the setup function which starts baobab)
    2) get the test into the required state for this test. ( so using functions outside the test function )
    3) once in the required state for the test, assert what we want to test. as the last part of the test function

take a look at the gedit test as an example the test are only testing one thing only then move on to the next test.

so as an example you could break tests up like

1) test we can open home folder ( but we do not check the folder is being displayed properly thats another test)
2) test that the tree map diplays the home folder
3) test trying to open a folder that doesn't exist.

Each test should not depend on any other test. So create functions of common code that you can use to get a test to its 'required state'

I hope this is of some help