Merge lp:~mikefletcher/mago/gnome-screenshot into lp:~mago-contributors/mago/mago-1.0
- gnome-screenshot
- Merge into mago-1.0
Status: | Merged |
---|---|
Merged at revision: | not available |
Proposed branch: | lp:~mikefletcher/mago/gnome-screenshot |
Merge into: | lp:~mago-contributors/mago/mago-1.0 |
Diff against target: | None lines |
To merge this branch: | bzr merge lp:~mikefletcher/mago/gnome-screenshot |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Ara Pulido | Approve | ||
Michael Fletcher (community) | Abstain | ||
Joker Wild (community) | in process test | Approve | |
James Tatum | Approve | ||
Nagappan Alagappan | Approve | ||
Review via email: mp+10684@code.launchpad.net |
Commit message
Added a basic test suite for gnome-screenshot. I followed the existing tests documented at http://
Description of the change
Michael Fletcher (mikefletcher) wrote : | # |
Joker Wild (lajjr-deactivatedaccount) wrote : | # |
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
Michael Fletcher (mikefletcher) wrote : | # |
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) wrote : | # |
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.
Michael Fletcher (mikefletcher) wrote : | # |
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:/
are the owner of...
James Tatum (jtatum) wrote : | # |
It looks good overall. The meat of the LDTP commands in mago/applicatio
James Tatum (jtatum) wrote : | # |
Sorry, one additional note regarding style, I notice that most of the mago/applicatio
Michael Fletcher (mikefletcher) wrote : | # |
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.
Nagappan Alagappan (nagappan) wrote : | # |
> 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
Nagappan Alagappan (nagappan) wrote : | # |
> Am I missing something ? I could not see any new diff in this review.
jtatum gave me the link - http://
Its nice to see the exceptions are caught in each and every line. But this will make the script writer job more difficult !?!
Thanks
James Tatum (jtatum) wrote : | # |
+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.
Michael Fletcher (mikefletcher) wrote : | # |
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) wrote : | # |
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 :).
- 110. By Michael Fletcher
-
Updated test to work with 9.10. It will now fail with 9.04.
- 111. By Michael Fletcher
-
Fixed silly mistake. I should have run the tests before I commited.
Joker Wild (lajjr-deactivatedaccount) wrote : | # |
With changes in place very good.
Michael Fletcher (mikefletcher) : | # |
Javier Collado (javier.collado) wrote : | # |
> 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:/
Look at comment #1 for a tool that is supposed to help to workaround it.
- 112. By Michael Fletcher
-
Reformatted and updated the documentation for epydoc.
Michael Fletcher (mikefletcher) wrote : | # |
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) wrote : | # |
> 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) wrote : | # |
I think it is ready to be merged. I am waiting for Michael's reply to an email I sent him, before commiting
Preview Diff
1 | === added directory 'gnome-screenshot' |
2 | === added file 'gnome-screenshot/take_screenshot.py' |
3 | --- gnome-screenshot/take_screenshot.py 1970-01-01 00:00:00 +0000 |
4 | +++ gnome-screenshot/take_screenshot.py 2009-08-25 18:33:48 +0000 |
5 | @@ -0,0 +1,48 @@ |
6 | +import os |
7 | +from mago.test_suite.gnome import GnomeScreenshotTestSuite |
8 | + |
9 | +class TakeScreenshot(GnomeScreenshotTestSuite): |
10 | + def grab_desktop(self, delay=5, file_name='baz.png'): |
11 | + self.application.open() |
12 | + self.application.take_whole_desktop_screenshot(delay) |
13 | + |
14 | + self.application.save_to_file(file_name) |
15 | + |
16 | + file_path = os.getenv('HOME') + '/' + file_name |
17 | + |
18 | + if os.access(file_path, os.F_OK): |
19 | + os.remove(file_path) |
20 | + else: |
21 | + raise AssertionError('Screenshot ' + file_path + ' was not created') |
22 | + |
23 | + def grab_window(self, delay=5, file_name='baz.png'): |
24 | + self.application.open() |
25 | + |
26 | + self.application.take_current_window_screenshot(delay) |
27 | + |
28 | + self.application.save_to_file(file_name) |
29 | + |
30 | + file_path = os.getenv('HOME') + '/' + file_name |
31 | + |
32 | + if os.access(file_path, os.F_OK): |
33 | + os.remove(file_path) |
34 | + else: |
35 | + raise AssertionError('Screenshot ' + file_path + ' was not created') |
36 | + |
37 | + def grab_area(self, delay=5, file_name='baz.png'): |
38 | + self.application.open() |
39 | + |
40 | + self.application.grab_selected_area(delay) |
41 | + |
42 | + self.application.save_to_file(file_name) |
43 | + |
44 | + file_path = os.getenv('HOME') + '/' + file_name |
45 | + |
46 | + if os.access(file_path, os.F_OK): |
47 | + os.remove(file_path) |
48 | + else: |
49 | + raise AssertionError('Screenshot ' + file_path + ' was not created') |
50 | + |
51 | +if __name__ == "__main__": |
52 | + test = TakeScreenshot() |
53 | + test.run() |
54 | |
55 | === added file 'gnome-screenshot/take_screenshot.xml' |
56 | --- gnome-screenshot/take_screenshot.xml 1970-01-01 00:00:00 +0000 |
57 | +++ gnome-screenshot/take_screenshot.xml 2009-08-25 18:33:48 +0000 |
58 | @@ -0,0 +1,25 @@ |
59 | +<?xml version="1.0"?> |
60 | +<suite name="take screenshot"> |
61 | + <class>take_screenshot.TakeScreenshot</class> |
62 | + <description> |
63 | + Tests which verify the Take Screenshot functionality. |
64 | + </description> |
65 | + <case name="Grab Desktop"> |
66 | + <method>grab_desktop</method> |
67 | + <description> |
68 | + Grab a screenshot of the desktop after a delay. |
69 | + </description> |
70 | + </case> |
71 | + <case name="Grab Window"> |
72 | + <method>grab_window</method> |
73 | + <description> |
74 | + Grab a screenshot of the current window after a delay. |
75 | + </description> |
76 | + </case> |
77 | + <case name="Grab Area"> |
78 | + <method>grab_area</method> |
79 | + <description> |
80 | + Grab a screenshot of the select area after a delay. |
81 | + </description> |
82 | + </case> |
83 | +</suite> |
84 | |
85 | === modified file 'mago/application/gnome.py' |
86 | --- mago/application/gnome.py 2009-07-10 09:24:57 +0000 |
87 | +++ mago/application/gnome.py 2009-08-25 18:33:48 +0000 |
88 | @@ -4,9 +4,98 @@ |
89 | The gnome module provides wrappers for LDTP to make the write of Gnome tests easier. |
90 | """ |
91 | import ooldtp |
92 | -import ldtp |
93 | +import ldtp |
94 | from .main import Application |
95 | |
96 | +class GnomeScreenshot(Application): |
97 | + LAUNCHER = 'gnome-screenshot' |
98 | + LAUNCHER_ARGS = ['--interactive'] |
99 | + CLOSE_TYPE = 'button' |
100 | + CLOSE_NAME = 'btnCancel' |
101 | + |
102 | + WINDOW = 'dlgTakeScreenshot' |
103 | + |
104 | + GRAB_AFTER_DELAY_FIELD = 'sbtnGrabafteradelayof' |
105 | + GRAB_THE_WHOLE_DESKTOP_RADIO = 'rbtnGrabthewholedesktop' |
106 | + GRAB_THE_CURRENT_WINDOW_RADIO = 'rbtnGrabthecurrentwindow' |
107 | + GRAB_A_SELECTED_AREA = 'rbtnGrabaselectedarea' |
108 | + |
109 | + TAKE_SCREENSHOT_BUTTON = 'btnTakeScreenshot' |
110 | + |
111 | + SAVE_FILE_WINDOW = 'dlgSaveScreenshot' |
112 | + SAVE_FILE_BUTTON = 'btnSave' |
113 | + SAVE_FILE_NAME_TEXT = 'txtName' |
114 | + SAVE_FILE_FOLDER_DROPDOWN = 'cbSaveFile' |
115 | + |
116 | + def __init__(self): |
117 | + Application.__init__(self) |
118 | + |
119 | + """ |
120 | + Complete the "Take Screenshot" dialog box by choosing whole desktop and |
121 | + clicking Take Screenshot. |
122 | + |
123 | + You must have started the application already. |
124 | + """ |
125 | + def take_whole_desktop_screenshot(self, delay): |
126 | + ldtp.click(self.WINDOW, self.GRAB_THE_WHOLE_DESKTOP_RADIO) |
127 | + |
128 | + ldtp.setvalue (self.WINDOW, self.GRAB_AFTER_DELAY_FIELD, delay) |
129 | + |
130 | + ldtp.click(self.WINDOW, self.TAKE_SCREENSHOT_BUTTON) |
131 | + |
132 | + ldtp.waittillguinotexist(self.WINDOW) |
133 | + |
134 | + """ |
135 | + Complete the "Take Screenshot" dialog box by choosing current window and |
136 | + clicking Take Screenshot. |
137 | + |
138 | + You must have started the application already. |
139 | + """ |
140 | + def take_current_window_screenshot(self, delay): |
141 | + ldtp.click(self.WINDOW, self.GRAB_THE_CURRENT_WINDOW_RADIO) |
142 | + |
143 | + ldtp.setvalue (self.WINDOW, self.GRAB_AFTER_DELAY_FIELD, delay) |
144 | + |
145 | + ldtp.click(self.WINDOW, self.TAKE_SCREENSHOT_BUTTON) |
146 | + |
147 | + ldtp.waittillguinotexist(self.WINDOW) |
148 | + |
149 | + """ |
150 | + Complete the "Take Screenshot" dialog box by choosing selected area and |
151 | + clicking Take Screenshot. |
152 | + |
153 | + You must have started the application already. |
154 | + """ |
155 | + def grab_selected_area(self, delay): |
156 | + ldtp.click(self.WINDOW, self.GRAB_A_SELECTED_AREA) |
157 | + |
158 | + ldtp.setvalue (self.WINDOW, self.GRAB_AFTER_DELAY_FIELD, delay) |
159 | + |
160 | + ldtp.click(self.WINDOW, self.TAKE_SCREENSHOT_BUTTON) |
161 | + |
162 | + ldtp.waittillguinotexist(self.WINDOW) |
163 | + |
164 | + ldtp.generatemouseevent(100,100,'b1p') |
165 | + ldtp.generatemouseevent(200,200,'abs') |
166 | + ldtp.generatemouseevent(200,200,'b1r') |
167 | + |
168 | + """ |
169 | + Complete the save to file dialog. You must have completed a grab_ already |
170 | + """ |
171 | + def save_to_file(self, filename): |
172 | + ldtp.waittillguiexist(self.SAVE_FILE_WINDOW) |
173 | + |
174 | + ldtp.settextvalue(self.SAVE_FILE_WINDOW, self.SAVE_FILE_NAME_TEXT, filename) |
175 | + |
176 | + # There is a bug in gnome-screenshot that prevents the combobox |
177 | + # name from being used. There is only one combo box so its easy to |
178 | + # select it by index. |
179 | + ldtp.comboselectindex(self.SAVE_FILE_WINDOW, 'cbo#0', 0) |
180 | + |
181 | + ldtp.click(self.SAVE_FILE_WINDOW, self.SAVE_FILE_BUTTON) |
182 | + |
183 | + ldtp.waittillguinotexist(self.SAVE_FILE_WINDOW) |
184 | + |
185 | class Seahorse(Application): |
186 | """ |
187 | Seahorse manages the Seahorse application. |
188 | |
189 | === modified file 'mago/application/main.py' |
190 | --- mago/application/main.py 2009-07-29 11:14:24 +0000 |
191 | +++ mago/application/main.py 2009-08-20 02:40:51 +0000 |
192 | @@ -10,13 +10,16 @@ |
193 | Superclass for the rest of the applications |
194 | |
195 | Constants that should be defined in the classes that inherit from this class |
196 | - LAUNCHER: Argument to be passed when launching the application through ldtp.launchapp |
197 | + LAUNCHER: Command to launching the application through ldtp.launchapp |
198 | + LAUNCHER_ARGS: Arguments to pass when launching the application through |
199 | + ldtp.launchapp. |
200 | WINDOW: Top application frame pattern using ldtp syntax |
201 | CLOSE_TYPE: Close object type (one of 'menu' or 'button') |
202 | CLOSE_NAME: Close object name |
203 | """ |
204 | CLOSE_TYPE = 'menu' |
205 | CLOSE_NAME = 'mnuQuit' |
206 | + LAUNCHER_ARGS = [''] |
207 | WINDOW = '' |
208 | TOP_PANEL = 'frmTopExpandedEdgePanel' |
209 | |
210 | @@ -68,7 +71,7 @@ |
211 | |
212 | """ |
213 | self._enable_a11y(True) |
214 | - ldtp.launchapp(self.LAUNCHER) |
215 | + ldtp.launchapp(self.LAUNCHER, arg=self.LAUNCHER_ARGS) |
216 | self._enable_a11y(False) |
217 | |
218 | ldtp.wait(2) |
219 | |
220 | === modified file 'mago/test_suite/gnome.py' |
221 | --- mago/test_suite/gnome.py 2009-05-27 10:45:38 +0000 |
222 | +++ mago/test_suite/gnome.py 2009-08-25 18:33:48 +0000 |
223 | @@ -4,7 +4,10 @@ |
224 | """ |
225 | import ldtp, ooldtp |
226 | from .main import SingleApplicationTestSuite |
227 | -from ..application.gnome import Application, Seahorse, GEdit |
228 | +from ..application.gnome import Application, GnomeScreenshot, Seahorse, GEdit |
229 | + |
230 | +class GnomeScreenshotTestSuite(SingleApplicationTestSuite): |
231 | + APPLICATION_FACTORY = GnomeScreenshot |
232 | |
233 | class SeahorseTestSuite(SingleApplicationTestSuite): |
234 | """ |
This is a basic test suite for gnome-screenshot. I followed the existings tests documented at http:// testcases. qa.ubuntu. com/Application s/TakeScreensho t.
Please be gentle. This is my first attempt at merging branches with launchpad. I might have totally messed up the branch or merge.