Merge lp:~mikefletcher/mago/gnome-screenshot into lp:~mago-contributors/mago/mago-1.0

[Michael Fletcher]

Added a basic test suite for gnome-screenshot. I followed the existing tests documented at http://testcases.qa.ubuntu.com/Applications/TakeScreenshot.

[Michael Fletcher (mikefletcher)]

This is a basic test suite for gnome-screenshot. I followed the existings tests documented at http://testcases.qa.ubuntu.com/Applications/TakeScreenshot.

Please be gentle. This is my first attempt at merging branches with launchpad. I might have totally messed up the branch or merge.

[Joker Wild (lajjr-deactivatedaccount)]

Michael,
Were you going to use mouse event to do this I looked at the source it seemed you started to add pointer. Then you removed it this is nice I see it grabbed a screen shot in my test it works well, but just curious if you were going to use a mouse event maybe use it to create an area to grab.

Regards,
Leo Jackson
lajjr

review: Needs Information (in process test)

[Michael Fletcher (mikefletcher)]

Hi.

> Were you going to use mouse event to do this I looked at the source it seemed
> you started to add pointer.
gnome-screenshot has an option to include the mouse pointer when taking the screenshot. 'include_pointer' was going to be part of those tests. However I thought I should "release early, release often" so I took those unfinished parts out.

> Then you removed it this is nice I see it grabbed
> a screen shot in my test it works well, but just curious if you were going to
> use a mouse event maybe use it to create an area to grab.
I did implement the area grab (with help from Nagappan). Its in gnome.py around line 67.

[Nagappan Alagappan (nagappan)]

It will be nice to move the screenshot file name to take_screenshot.xml

In gnome.py line # 164-166 coordinate information are hard coded. Will this work on all test environment ? I have a dual screen monitor ;-) I recommend to use getobjectsize and then use the relative x, y coordinates.

review: Needs Fixing

[Michael Fletcher (mikefletcher)]

Hi.

I will put all the arguments into the .xml file.

Gnomescreenshot is asking you to select an area of your entire screen. As
long as your screen is larger that 200x200 pixels this should be fine. In
either case you need a window for getobjectsize and in this test I don't
have one.

... sent from my Android Phone

On Aug 25, 2009 3:57 PM, "Nagappan Alagappan" <email address hidden> wrote:

Review: Needs Fixing
It will be nice to move the screenshot file name to take_screenshot.xml

In gnome.py line # 164-166 coordinate information are hard coded. Will this
work on all test environment ? I have a dual screen monitor ;-) I recommend
to use getobjectsize and then use the relative x, y coordinates.

--
https://code.launchpad.net/~mikefletcher/mago/gnome-screenshot/+merge/10684You
are the owner of...

[James Tatum (jtatum)]

It looks good overall. The meat of the LDTP commands in mago/application/gnome.py are all wrapped in try/catch blocks to provide a helpful failure description for the log file. +1 to the comments for moving as many parameters as possible to XML.

review: Needs Fixing

[James Tatum (jtatum)]

Sorry, one additional note regarding style, I notice that most of the mago/application/gnome.py applications are written as ooldtp, and the proposed code for gnome-screenshot is procedural. I'm not sure if that's a factor since I'm new here myself :) Just pointing it out.

[Michael Fletcher (mikefletcher)]

Hi. I made all of the requested changes to the gnome-screenshot tests.

* ooldtp is being used
* try: except: should indicate where something fails
* the arguments are being specified using the xml files.

review: Needs Resubmitting

[Nagappan Alagappan (nagappan)]

> Hi. I made all of the requested changes to the gnome-screenshot tests.
>
> * ooldtp is being used
> * try: except: should indicate where something fails
> * the arguments are being specified using the xml files.

Am I missing something ? I could not see any new diff in this review.

Thanks

review: Needs Information

[Nagappan Alagappan (nagappan)]

> Am I missing something ? I could not see any new diff in this review.

jtatum gave me the link - http://bazaar.launchpad.net/~mikefletcher/mago/gnome-screenshot/revision/109

Its nice to see the exceptions are caught in each and every line. But this will make the script writer job more difficult !?!

Thanks

review: Approve

[James Tatum (jtatum)]

+1 - that is a lot of exceptions :) It can always be condensed later of course. I think the balance in e.g. seahorse in gnome.py is pretty good.

review: Approve

[Michael Fletcher (mikefletcher)]

Sorry about the change-of-status spam. I had heard that changing the status of the merge proposal would regenerate the diff but it didn't work.

[Michael Fletcher (mikefletcher)]

I think the exception handling is a bit much. However it is consistent with
the rest of the code and i'm not interested in rocking.

The ldtp exception says
    u'Unable to find object name in application map'

And the exceptions we raise says
    'Window dlgSaveScreenshot did not appear'

I think a good solution would be to add information to the ldtp exceptions
so that the mago exception handling is superfluous. But I wont mention it
again until I have written a patch :).

[lp:~mikefletcher/mago/gnome-screenshot]

110. By Michael Fletcher on 2009-08-26

Updated test to work with 9.10. It will now fail with 9.04.

111. By Michael Fletcher on 2009-08-26

Fixed silly mistake. I should have run the tests before I commited.

[Joker Wild (lajjr-deactivatedaccount)]

With changes in place very good.

review: Approve (in process test)

[Michael Fletcher (mikefletcher)]

review: Abstain

[Javier Collado (javier.collado)]

> Sorry about the change-of-status spam. I had heard that changing the status
> of the merge proposal would regenerate the diff but it didn't work.

I also noticed this in other reviews.

It seems like this is known problem that is going to be solved:
https://bugs.launchpad.net/launchpad-code/+bug/338002

Look at comment #1 for a tool that is supposed to help to workaround it.

[lp:~mikefletcher/mago/gnome-screenshot]

112. By Michael Fletcher on 2009-09-02

Reformatted and updated the documentation for epydoc.

[Michael Fletcher (mikefletcher)]

I pushed some fixes to the documentation strings so they get picked
up by epydoc.

Reminder: The diff does not get updated so you will have to look at
the individual changes (changes 106 -> 112).

[Ara Pulido (ara)]

> I think the exception handling is a bit much. However it is consistent with
> the rest of the code and i'm not interested in rocking.
>
>
> The ldtp exception says
> u'Unable to find object name in application map'
>
> And the exceptions we raise says
> 'Window dlgSaveScreenshot did not appear'
>
>
> I think a good solution would be to add information to the ldtp exceptions
> so that the mago exception handling is superfluous. But I wont mention it
> again until I have written a patch :).

I agree with you, Michael, although I was the one that started the try/catch madness, I never liked it myself.

[Ara Pulido (ara)]

I think it is ready to be merged. I am waiting for Michael's reply to an email I sent him, before commiting

review: Approve

=== added directory 'gnome-screenshot'
=== added file 'gnome-screenshot/take_screenshot.py'
--- gnome-screenshot/take_screenshot.py	1970-01-01 00:00:00 +0000
+++ gnome-screenshot/take_screenshot.py	2009-08-25 18:33:48 +0000
@@ -0,0 +1,48 @@
+import os
+from mago.test_suite.gnome import GnomeScreenshotTestSuite
+
+class TakeScreenshot(GnomeScreenshotTestSuite):
+    def grab_desktop(self, delay=5, file_name='baz.png'):
+        self.application.open()
+        self.application.take_whole_desktop_screenshot(delay)
+
+        self.application.save_to_file(file_name)
+
+        file_path = os.getenv('HOME') + '/' + file_name
+        
+        if os.access(file_path, os.F_OK):
+            os.remove(file_path) 
+        else:
+            raise AssertionError('Screenshot ' + file_path + ' was not created')
+
+    def grab_window(self, delay=5, file_name='baz.png'):
+        self.application.open()
+
+        self.application.take_current_window_screenshot(delay)
+
+        self.application.save_to_file(file_name)
+
+        file_path = os.getenv('HOME') + '/' + file_name
+        
+        if os.access(file_path, os.F_OK):
+            os.remove(file_path) 
+        else:
+            raise AssertionError('Screenshot ' + file_path + ' was not created')
+
+    def grab_area(self, delay=5, file_name='baz.png'):
+        self.application.open()
+
+        self.application.grab_selected_area(delay)
+
+        self.application.save_to_file(file_name)
+
+        file_path = os.getenv('HOME') + '/' + file_name
+        
+        if os.access(file_path, os.F_OK):
+            os.remove(file_path) 
+        else:
+            raise AssertionError('Screenshot ' + file_path + ' was not created')
+
+if __name__ == "__main__":
+    test = TakeScreenshot()
+    test.run()
 
=== added file 'gnome-screenshot/take_screenshot.xml'
--- gnome-screenshot/take_screenshot.xml	1970-01-01 00:00:00 +0000
+++ gnome-screenshot/take_screenshot.xml	2009-08-25 18:33:48 +0000
@@ -0,0 +1,25 @@
+<?xml version="1.0"?>
+<suite name="take screenshot">
+  <class>take_screenshot.TakeScreenshot</class>
+  <description>
+    Tests which verify the Take Screenshot functionality.
+  </description>
+  <case name="Grab Desktop">
+    <method>grab_desktop</method>
+    <description>
+        Grab a screenshot of the desktop after a delay.
+    </description>
+  </case>
+  <case name="Grab Window">
+    <method>grab_window</method>
+    <description>
+        Grab a screenshot of the current window after a delay.
+    </description>
+  </case>
+  <case name="Grab Area">
+    <method>grab_area</method>
+    <description>
+        Grab a screenshot of the select area after a delay.
+    </description>
+  </case>
+</suite>
 
=== modified file 'mago/application/gnome.py'
--- mago/application/gnome.py	2009-07-10 09:24:57 +0000
+++ mago/application/gnome.py	2009-08-25 18:33:48 +0000
@@ -4,9 +4,98 @@
 The gnome module provides wrappers for LDTP to make the write of Gnome tests easier. 
 """
 import ooldtp
-import ldtp 
+import ldtp
 from .main import Application
 
+class GnomeScreenshot(Application):
+    LAUNCHER = 'gnome-screenshot'
+    LAUNCHER_ARGS = ['--interactive']
+    CLOSE_TYPE = 'button'
+    CLOSE_NAME = 'btnCancel'
+
+    WINDOW = 'dlgTakeScreenshot'
+    
+    GRAB_AFTER_DELAY_FIELD = 'sbtnGrabafteradelayof'
+    GRAB_THE_WHOLE_DESKTOP_RADIO = 'rbtnGrabthewholedesktop'
+    GRAB_THE_CURRENT_WINDOW_RADIO = 'rbtnGrabthecurrentwindow'
+    GRAB_A_SELECTED_AREA = 'rbtnGrabaselectedarea'
+
+    TAKE_SCREENSHOT_BUTTON = 'btnTakeScreenshot'
+
+    SAVE_FILE_WINDOW = 'dlgSaveScreenshot'
+    SAVE_FILE_BUTTON = 'btnSave'
+    SAVE_FILE_NAME_TEXT = 'txtName'
+    SAVE_FILE_FOLDER_DROPDOWN = 'cbSaveFile'
+
+    def __init__(self):
+        Application.__init__(self)
+
+    """
+    Complete the "Take Screenshot" dialog box by choosing whole desktop and
+    clicking Take Screenshot.
+
+    You must have started the application already.
+    """
+    def take_whole_desktop_screenshot(self, delay):
+        ldtp.click(self.WINDOW, self.GRAB_THE_WHOLE_DESKTOP_RADIO)
+
+        ldtp.setvalue (self.WINDOW, self.GRAB_AFTER_DELAY_FIELD, delay)
+
+        ldtp.click(self.WINDOW, self.TAKE_SCREENSHOT_BUTTON)
+
+        ldtp.waittillguinotexist(self.WINDOW)
+
+    """
+    Complete the "Take Screenshot" dialog box by choosing current window and
+    clicking Take Screenshot.
+
+    You must have started the application already.
+    """
+    def take_current_window_screenshot(self, delay):
+        ldtp.click(self.WINDOW, self.GRAB_THE_CURRENT_WINDOW_RADIO)
+
+        ldtp.setvalue (self.WINDOW, self.GRAB_AFTER_DELAY_FIELD, delay)
+
+        ldtp.click(self.WINDOW, self.TAKE_SCREENSHOT_BUTTON)
+
+        ldtp.waittillguinotexist(self.WINDOW)
+
+    """
+    Complete the "Take Screenshot" dialog box by choosing selected area and
+    clicking Take Screenshot.
+
+    You must have started the application already.
+    """
+    def grab_selected_area(self, delay):
+        ldtp.click(self.WINDOW, self.GRAB_A_SELECTED_AREA)
+
+        ldtp.setvalue (self.WINDOW, self.GRAB_AFTER_DELAY_FIELD, delay)
+
+        ldtp.click(self.WINDOW, self.TAKE_SCREENSHOT_BUTTON)
+
+        ldtp.waittillguinotexist(self.WINDOW)
+
+        ldtp.generatemouseevent(100,100,'b1p')
+        ldtp.generatemouseevent(200,200,'abs')
+        ldtp.generatemouseevent(200,200,'b1r')
+    
+    """
+    Complete the save to file dialog.  You must have completed a grab_ already
+    """
+    def save_to_file(self, filename):
+        ldtp.waittillguiexist(self.SAVE_FILE_WINDOW)
+
+        ldtp.settextvalue(self.SAVE_FILE_WINDOW, self.SAVE_FILE_NAME_TEXT, filename)
+
+        # There is a bug in gnome-screenshot that prevents the combobox 
+        # name from being used.  There is only one combo box so its easy to 
+        # select it by index.
+        ldtp.comboselectindex(self.SAVE_FILE_WINDOW, 'cbo#0', 0)
+
+        ldtp.click(self.SAVE_FILE_WINDOW, self.SAVE_FILE_BUTTON)
+
+        ldtp.waittillguinotexist(self.SAVE_FILE_WINDOW)
+
 class Seahorse(Application):
     """
     Seahorse manages the Seahorse application.
 
=== modified file 'mago/application/main.py'
--- mago/application/main.py	2009-07-29 11:14:24 +0000
+++ mago/application/main.py	2009-08-20 02:40:51 +0000
@@ -10,13 +10,16 @@
     Superclass for the rest of the applications
 
     Constants that should be defined in the classes that inherit from this class
-    LAUNCHER: Argument to be passed when launching the application through ldtp.launchapp
+    LAUNCHER: Command to launching the application through ldtp.launchapp
+    LAUNCHER_ARGS: Arguments to pass when launching the application through
+                   ldtp.launchapp.
     WINDOW: Top application frame pattern using ldtp syntax
     CLOSE_TYPE: Close object type (one of 'menu' or 'button')
     CLOSE_NAME: Close object name
     """
     CLOSE_TYPE = 'menu'
     CLOSE_NAME = 'mnuQuit'
+    LAUNCHER_ARGS = ['']
     WINDOW     = ''
     TOP_PANEL = 'frmTopExpandedEdgePanel'
 
@@ -68,7 +71,7 @@
          
         """
         self._enable_a11y(True)
-        ldtp.launchapp(self.LAUNCHER)
+        ldtp.launchapp(self.LAUNCHER, arg=self.LAUNCHER_ARGS)
         self._enable_a11y(False)
 
         ldtp.wait(2)
 
=== modified file 'mago/test_suite/gnome.py'
--- mago/test_suite/gnome.py	2009-05-27 10:45:38 +0000
+++ mago/test_suite/gnome.py	2009-08-25 18:33:48 +0000
@@ -4,7 +4,10 @@
 """
 import ldtp, ooldtp
 from .main import SingleApplicationTestSuite
-from ..application.gnome import Application, Seahorse, GEdit
+from ..application.gnome import Application, GnomeScreenshot, Seahorse, GEdit
+
+class GnomeScreenshotTestSuite(SingleApplicationTestSuite):
+    APPLICATION_FACTORY = GnomeScreenshot   
 
 class SeahorseTestSuite(SingleApplicationTestSuite):
     """