Merge lp://staging/~senan/ubuntu-autopilot-tests/DiskUsageAnalyser into lp://staging/ubuntu-autopilot-tests

First off this is a great start :-) Well Done. Its always the trickiest part learning what you can and can't do with autopilot.

So my first thoughts are (and keeping in mind the near release of autopilot 1.4),

1) the test window title failed on my box. Due to having a different locale where analyzer is actually analyser. So to make this work you would probably be better testing the title with something like Contains('Disk Usage'). Then we miss the strange locale issues.

Note that if you wanted the test to pass on any locale you could assertThat(title, NotEquals(None)) and then assertIsInstance(title, unicode). This will test that the title contains text (we don't know what the title text will be, but we can test the title is there and that its unicode.

2) the other 3 tests are very similar so you could probably break them up a little. My first point with this is you are finishing each test outside of the test function with the self.disk_usage_display_common() call. Ideally you should be finishing each test with asserts as the last lines inside your test function. This way you can see the intended end result of the test i.e "the treeviewmap and ringmap should display a 'x' directory/folder" try and do the setup type parts to get you to the point you need outside the test function and then come back once setup to test the thing you are actually testing

another thing when entering /home in the file chooser dialog you should use the provided function in the keyboard class 'focused_type' so for example once you have clicked the toggle button and the entry is visible you could use something like this

entry = self.app.select_single('GtkFileChooserEntry')
with self.keyboard.focused_type(entry) as kb:
    kb.type('/home')
    self.assertThat(entry.text, Equals('/home'))
    kb.press_and_release('Enter')

This will get the entries focus and type the text see http://unity.ubuntu.com/autopilot/api/input.html#autopilot.input.Keyboard.focused_type

There is also some changes needed to be made for 1.4 if you check this link about boolean properties are now represented correctly and not 1/0. http://unity.ubuntu.com/autopilot/porting/porting.html#gtk-tests-and-boolean-parameters

I'll leave it there for now and see how we fair once these changes are made.

Feel free to ping myself or balloons if your are getting stuck :-)

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

Senan

so the test_window_title is still failing for me due to the locale issue with the spelling of analyser so i would just remove that and assert 'Disk Usage '.

Also when entering text you need explicitly make sure you get the text entry focus, by either clicking on it assert entry.is_focus = True and then type or you can use the focused_type context manager. e.g currenlty you have this

111 + self.create_toggle_button = self.app.select_single('GtkToggleButton',tooltip_text=u'Type a file name')
112 + self.pointing_device.move_to_object(self.create_toggle_button)
113 + self.pointing_device.click()
114 + self.keyboard.type(key_input)

Which once you have clicked the toggle button we do not actually know if the entry is now visable and has focus before typing. So after the clicking the toggle button you need to

1) assert entry.visible == True

2) either click on the entry and assert entry.is_focus == True before typing
or use the focused_type context manager (which i prefer) which looks something like

entry = self.app.select_single('GtkFileChooserEntry')
 with self.keyboard.focused_type(entry) as kb:
     kb.type(key_input)
     self.assertThat(entry.text, Equals(key_input)) (after typing assert it type correctly)
     kb.press_and_release('Enter')

3) test you have ended where you expected :-)

So anywhere in your tests you are entering text you need to do one of the above so we know the entry has focus if we can't get focus then we have a bug :-)

One last thing all boolean values your test that are using ints need to be changed for autopilot 1.4 i.e

self.assertThat(window.visible, Equals(1)) will become self.assertThat(window.visible, Equals(True)) and all 0 values become False. If you are on saucy install autopilot from the ppa:autopilot/ppa and you will get autopilot 1.4
We need to make our tests compliant with 1.4 as soon as possible.

THank you for iterating over these tests, your doing a great job and soon you will get into the swing of it, just hang in there :-)

Dan

Hey senan, i'm still looking into the problems we were having last night. I am waiting on a reply from someone who may no more so as soon as hear anything we will move onwards with this. Sorry about the delay, sometimes these little niggles need sorting first :-)

Hope you are well

Dan

Thanks Dan
On 22 Nov 2013 16:36, "Dan Chapman" <email address hidden> wrote:

> Hey senan, i'm still looking into the problems we were having last night.
> I am waiting on a reply from someone who may no more so as soon as hear
> anything we will move onwards with this. Sorry about the delay, sometimes
> these little niggles need sorting first :-)
>
> Hope you are well
>
> Dan
> --
>
> https://code.launchpad.net/~senan/ubuntu-autopilot-tests/DiskUsageAnalyser/+merge/193087
> You are the owner of lp:~senan/ubuntu-autopilot-tests/DiskUsageAnalyser.
>

Senan hey you left too soon, ok ive been having a play and added something ive been working on for a little while (some helpful emulators :-) ), ive adapted your branch slightly here https://code.launchpad.net/~dpniel/+junk/baobab and it uses emulators for selecting items in tree so feel free to branch it have a play and see what you can come up with, take a while to look through emulators.py and see whats happening :-)

Hope this helps in the mean time till we can figure out what the difference is between desktop and sandbox

Dan

Hi Dan,

I've made some changes to the code in order to address the
filechooserdialog treeview click issue. http://paste.ubuntu.com/6612284/ .
But I am sure its not the best way to resolve the issue. But I am still not
able to identify the treeview. Looking forward to your review comments.

On Tue, Oct 29, 2013 at 10:01 PM, Prabhendu V Senan <<email address hidden>
> wrote:

> Prabhendu V Senan has proposed merging
> lp:~senan/ubuntu-autopilot-tests/DiskUsageAnalyser into
> lp:ubuntu-autopilot-tests.
>
> Requested reviews:
> Ubuntu Testcase Admins (ubuntu-testcase)
>
> For more details, see:
>
> https://code.launchpad.net/~senan/ubuntu-autopilot-tests/DiskUsageAnalyser/+merge/193087
> --
>
> https://code.launchpad.net/~senan/ubuntu-autopilot-tests/DiskUsageAnalyser/+merge/193087
> You are the owner of lp:~senan/ubuntu-autopilot-tests/DiskUsageAnalyser.
>
> === added directory 'ubuntu_autopilot_tests/DiskUsageAnalyser'
> === added file 'ubuntu_autopilot_tests/DiskUsageAnalyser/__init__.py'
> === added file
> 'ubuntu_autopilot_tests/DiskUsageAnalyser/test_diskUsageAnalyser.py'
> --- ubuntu_autopilot_tests/DiskUsageAnalyser/test_diskUsageAnalyser.py
> 1970-01-01 00:00:00 +0000
> +++ ubuntu_autopilot_tests/DiskUsageAnalyser/test_diskUsageAnalyser.py
> 2013-10-29 16:26:36 +0000
> @@ -0,0 +1,95 @@
> +from autopilot.testcase import AutopilotTestCase
> +from autopilot.matchers import Eventually
> +from testtools.matchers import Equals, Contains, NotEquals
> +
> +from autopilot.input import Mouse, Pointer
> +
> +import os
> +from time import sleep
> +
> +class DiskUsageAnalyserTests(AutopilotTestCase):
> +
> + def setUp(self):
> + super(DiskUsageAnalyserTests,self).setUp()
> + self.app = self.launch_test_application('baobab')
> + self.mainWindowTitle = self.app.select_single('BaobabWindow')
> + self.pointing_device = Pointer(Mouse.create())
> +
> + def test_window_title(self):
> + # Verify the Disk Usage Analyzer application Title
> +
> self.assertThat(self.mainWindowTitle.title,Eventually(Contains('Disk Usage
> Analyzer')))
> +
> + 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()
> + self.assertThat(lambda:
> self.app.select_single('GtkFileChooserDialog').visible,Eventually(Equals(1)))
> + dialog = self.app.select_single('GtkFileChooserDialog')
> + self.assertThat(dialog.title,Eventually(Equals('Select
> Folder')))
> + tree_view = dialog.select_many('GtkTreeView')[0]
> + self.pointing_device.click_object(tree_view)
> +
> + self.create_toggle_button =
> self.app.select_single('GtkToggleButton',tooltip_text=u'Type a file name')
> +
> self.po...

This is really making progress now, nice work. THere is just a couple more things that need tweaking,

1) The test script formatting is a little whacky, it looks like you have been using 8 space tab characters for part of it, ideally you should set this to 4 spaces.

2) When entering /home/ in the file chooser dialog it doesn't seem to close beacause of the autocompletion box being open, you need to make sure after typing either press 'Delete' so the autocompletion box closes or double click the _open button. It would also be good to assert that the PathBar in the dialog is in the /Home directory.

3) With regards to checking the ring and bar charts igf you look in vis their visible property is always set to true even when not visible, so asserting it's visible isn't gaining us anything. We should try and find another way to assert they are visible

If you can make these changes then i think we are nearly there :-) Nice One!

> This is really making progress now, nice work. THere is just a couple more
> things that need tweaking,

Thank you very much for the review :)
>
> 1) The test script formatting is a little whacky, it looks like you have been
> using 8 space tab characters for part of it, ideally you should set this to 4
> spaces.

I've fixed the spacing issue
>
> 2) When entering /home/ in the file chooser dialog it doesn't seem to close
> beacause of the autocompletion box being open, you need to make sure after
> typing either press 'Delete' so the autocompletion box closes or double click
> the _open button. It would also be good to assert that the PathBar in the
> dialog is in the /Home directory.

I've already implemented the code for removing auto-completion

editBox = self.app.select_single('GtkFileChooserEntry')
  #self.keyboard.focused_type(editBox)
  self.assertThat(editBox.is_focus,Equals(True))
  self.keyboard.type(key_input)
  sleep(2)
  self.keyboard.press_and_release('Delete')
  self.assertThat(editBox.text,Equals(key_input))

I am not sure why the auto-completion box is still showing even after this.. can you please check that. Also I didnt understand assert /Home in pathbar.PathBar in filechooserDlg or after clicking _open button
>
> 3) With regards to checking the ring and bar charts igf you look in vis their
> visible property is always set to true even when not visible, so asserting
> it's visible isn't gaining us anything. We should try and find another way to
> assert they are visible

I couldn't find any other propery to validate in this case..:( .. need your help
>
> If you can make these changes then i think we are nearly there :-) Nice One!

Senan
On 05/02/14 16:41, Prabhendu V Senan wrote:
>>
>>
>> I've already implemented the code for removing auto-completion
>>
>> editBox = self.app.select_single('GtkFileChooserEntry')
>> #self.keyboard.focused_type(editBox)
>> self.assertThat(editBox.is_focus,Equals(True))
>> self.keyboard.type(key_input)
>> sleep(2)
>> self.keyboard.press_and_release('Delete')
>> self.assertThat(editBox.text,Equals(key_input))
>>
>> I am not sure why the auto-completion box is still showing even after this.. can you please check that. Also I didnt understand assert /Home in pathbar.PathBar in filechooserDlg or after clicking _open button
This can be simple resolved by removing the trailing / so '/home/'
becomes '/home'. For the path bar it would be ideal to assert we are in
the required directory either by the pathbars label or stock icon
properties to determin we are in the correct place before clicking open
>> 3) With regards to checking the ring and bar charts igf you look in vis their
>> visible property is always set to true even when not visible, so asserting
>> it's visible isn't gaining us anything. We should try and find another way to
>> assert they are visible
> I couldn't find any other propery to validate in this case..:( .. need your help
I will need to investigate this further as it does seem quite tricky

can the sleeps be removed completely?
They tend to make tests brittle, and can generally always be replaced by another waiting mechanism.

check out the AutoPilot Docs... there is a section explaining `wait_for` and `eventually` for replacing `sleep`:

http://unity.ubuntu.com/autopilot/tutorial/good_tests.html#prefer-wait-for-and-eventually-to-sleep

p.s. let me know if you have any questions or need help refactoring... glad to lend a hand.
(cgoldberg on #freenode)

Overall, this has come a long way, good work!

I'd run pep8 and pyflakes (or just use flake8 to run them https://pypi.python.org/pypi/flake8) to help clean up the source and make sure things are good. For example, you can remove the sleep import now that you've removed the sleep statements ;-)

I notice you have calls like this

self.open_scan_folder_dialog()

followed by a check to ensure it becomes visible. It probably makes sense to move this check inside the function as well, so you can be assured the dialog is open. I would do this with all of your utility functions.

Some of your asserts are very hard to understand because they are so long. If they fail, it will be hard to understand why. So for example, instead of

self.assertThat(lambda:self.app.select_single('GdHeaderBar',BuilderName='result-header-bar').title,Eventually(Contains('home')))

Let's do

result_header = self.app.select_single('GdHeaderBar',BuilderName='result-header-bar')
self.assertThat(lambda: result_header.title,Eventually(Contains('home')))

or if the title isn't changing

result_header = self.app.select_single('GdHeaderBar',BuilderName='result-header-bar').title
self.assertThat(result_header,Contains('home'))

Finally, let's try and add some more comments as to what's going on. Docstrings (http://legacy.python.org/dev/peps/pep-0257/) are useful to explain what the test is trying to do. In simple language, add a docstring below each function explaining it's input/output and purpose.

All of these will make the code easier to read and understand. Thanks!

This looks much better, much easier to read and follow.

> This looks much better, much easier to read and follow.
Please let me know If I need to add anything to the test. If everything looks good, then I can start automating Deja-dup :D

Just adding some of my comments from IRC :-)

senan, run pep8 and pyflakes on the code and fix any errors.
<elfy> balloons: done
<balloons> senan, I was thinking I would change the name of the non-test functions as well, or move some of the utility functions to the emulator, but I'm reconsidering
<balloons> I think I might be ok with everything on that front
<balloons> senan, I suppose the other thing I would like to see is removing any commented out code, and add some inline coding comments to follow along (docstrings you added are wonderful thank you)

I would like to see the bug # added, as discussed on IRC. With that I approve.