New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate regression-documentation to SLED15 #4133
Conversation
9047ebd
to
8aabcaa
Compare
@@ -68,7 +68,8 @@ sub clean_shotwell { | |||
sub upload_libreoffice_specified_file { | |||
|
|||
x11_start_program('xterm'); | |||
assert_script_run("wget " . autoinst_url . "/data/x11regressions/ooo-test-doc-types.tar.bz2 -O /home/$username/Documents/ooo-test-doc-types.tar.bz2"); | |||
type_string_slow("wget " . autoinst_url . "/data/x11regressions/ooo-test-doc-types.tar.bz2 -O /home/$username/Documents/ooo-test-doc-types.tar.bz2"); | |||
send_key "ret"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you don't want to test the result of the wget? What happen if it fails?
And why should not do the 'return' inside type_string_slow
by adding '\n' at the end? (but I agreed, it works ;-))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the status for this? Is the wget can fail without any issue for the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ldevulder is right.
I consider this a bad idea. You are not checking the result of the wget and also type slower, "just to be safe"? If there is an openQA related problem why we need to type slower then we should be explicit about it not waste time unconditionally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ldevulder and @okurz
I've added '\n' at the end of "type_string_slow" and removed "send_key "ret";" based on the first comment. Also added "assert_screen("libreoffice-wget-success");" to check if wget is successful.
Since this PR has been merged. I will upload those changes in further PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should need to check the output of a script with a needle. This is what we have assert_script_run for. IMHO we should get rid of the type_string_slow here. It was added initially for the grub menue which has a limitation itself but within a full blown bash session there should be no problem to type with default speed unless we have a machine specific problem which should be solved with setting the test variable VNC_TYPING_LIMIT and not by typing slower in the test code
lib/x11regressiontest.pm
Outdated
@@ -111,7 +112,7 @@ sub cleanup_libreoffice_recent_file { | |||
} | |||
|
|||
sub open_libreoffice_options { | |||
if (is_tumbleweed) { | |||
if (is_tumbleweed or sle_version_at_least('15')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to test a SLE version you also need to use is_sle
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, please help to review.
@@ -28,7 +28,8 @@ sub run { | |||
send_key 'ctrl-ret'; #switch to link | |||
assert_screen 'gnote-note-start-here'; | |||
|
|||
wait_screen_change { send_key 'ctrl-tab' }; #jump to toolbar | |||
wait_screen_change { send_key 'ctrl-tab' }; #jump to toolbar | |||
wait_screen_change { send_key 'ctrl-tab' } if (sle_version_at_least('15')); #jump to toolbar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, is_sle
is needed to test a SLE version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, please help to review.
@@ -40,7 +41,7 @@ sub run { | |||
assert_screen 'gnote-what-link-here'; | |||
wait_screen_change { send_key 'esc' }; | |||
#close the note "Start Here" | |||
wait_screen_change { send_key 'ctrl-w' } if (!is_tumbleweed); | |||
wait_screen_change { send_key 'ctrl-w' } if (!is_tumbleweed && !sle_version_at_least('15')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above for is_sle
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, please help to review.
@@ -25,7 +26,8 @@ sub run { | |||
send_key "up"; | |||
type_string "new title-opensuse\n"; | |||
send_key "ctrl-tab"; #jump to toolbar | |||
send_key "ret"; #back to all notes interface | |||
send_key "ctrl-tab" if (sle_version_at_least('15')); #jump to toolbar for SLED15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above for is_sle
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, please help to review.
@@ -54,7 +54,7 @@ sub select_base_and_cleanup { | |||
sub run { | |||
my $self = shift; | |||
|
|||
if (!is_tumbleweed) { | |||
if (!is_tumbleweed && !sle_version_at_least('15')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above for is_sle
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I just fixed it, please help to review.
@@ -36,8 +36,13 @@ sub run { | |||
wait_still_screen; | |||
x11_start_program('oowriter'); | |||
send_key "alt-f"; | |||
# Because of bsc#1074057 alt-f is not working in libreoffice under wayland | |||
# use another way to replace alt-f in SLED15 | |||
if (sle_version_at_least('15')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above for is_sle
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I just fixed it, please help to review.
assert_screen 'oowriter-menus-file'; | ||
if (is_tumbleweed) { | ||
if (is_tumbleweed || sle_version_at_least('15')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above for is_sle
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I just fixed it, please help to review.
@@ -14,6 +14,7 @@ | |||
use base "x11regressiontest"; | |||
use strict; | |||
use testapi; | |||
use utils; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See remark about utils
vs version_utils
above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I just fixed it, please help to review.
@@ -22,8 +23,10 @@ sub run { | |||
assert_screen('test-desktop_mainmenu-1'); | |||
|
|||
# find the favorites button | |||
assert_and_click('application-menu-favorites'); | |||
assert_screen('menu-favorites-libreoffice'); | |||
if (!sle_version_at_least('15')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above for is_sle
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I just fixed it, please help to review.
8aabcaa
to
f4bb78e
Compare
Latest verification run: http://10.67.17.21/tests/223 |
send_key "ctrl-tab"; #jump to toolbar | ||
send_key "ret"; #back to all notes interface | ||
send_key "ctrl-tab"; #jump to toolbar | ||
send_key "ctrl-tab" if (is_sle && sle_version_at_least('15')); #jump to toolbar for SLED15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a send_key_until_needlematch 'ctrl-tab'
could not do the job here? Could be easier than using a version test (sorry, I didn't see this last time).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for pointing this out. Actually I was considering use send_key_until_needlematch 'ctrl-tab'
, and on the other hand I am trying to highlight the different behavior between SLED15 and earlier SLED version. It would be more clear to show the difference.
Please let me know if you still want to use send_key_until_needlematch 'ctrl-tab'
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK for me, will see if the merger will be ok with this. As I don't have merge rights, I'm only a young reviewer ;-)
@@ -1,6 +1,6 @@ | |||
# SUSE's openQA tests | |||
# | |||
# Copyright © 2016 SUSE LLC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
permission change is planned ? what the reason for this ?
(just merge @okurz PR where he fixes set of files to actually have 10644)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was planning to change the copyright and make it consistent with other files. Since the copyright in most other x11regression files is 2016 - 2017.
Sorry I forgot it's already 2018 not 2017.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the permission back to 10644
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed (again) in #4155
- This testsuite will be renamed as desktopapps-documentation and scheduled under Desktop Applications - desktopapps-documentation: BOOTFROM=c, REGRESSION=documentation, SLE_PRODUCT=sled, HDD_1=SLE-%VERSION%-%ARCH%-Build%BUILD%-sled-gnome.qcow2, START_AFTER_TEST=create_hdd_sled_gnome - Update x11regressiontest.pm to resolve the input string issue - Update 3 test cases for gnote - Update 5 test cases for libreoffice see also poo#27169
f4bb78e
to
250c262
Compare
Hmmm the Travis error is strange: it'a about a file you didn't change, but is it a new file? |
Travis failed with the below errors which were not related to this PR. |
issue catched by Travis , already fixed by this PR 1820398#diff-e60b3146ece9c3977023f86fa81605d7 |
BOOTFROM=c, REGRESSION=documentation, SLE_PRODUCT=sled,
HDD_1=SLE-%VERSION%-%ARCH%-Build%BUILD%-sled-gnome.qcow2,
START_AFTER_TEST=create_hdd_sled_gnome
see also poo#27169