Skip to content
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

Add assert_screen_change #689

Merged
merged 1 commit into from
Jan 12, 2017

Conversation

okurz
Copy link
Member

@okurz okurz commented Jan 5, 2017

Variant of wait_screen_change but dying on no screen change.

@okurz okurz force-pushed the feature/assert_screen_change branch from 15e1f7f to c082982 Compare January 6, 2017 10:00
@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 49.832% when pulling c082982 on okurz:feature/assert_screen_change into 2040b64 on os-autoinst:master.

@coolo
Copy link
Contributor

coolo commented Jan 11, 2017

I think this CODEREF is a bit too perl specific. I suggest to add an example for perl beginners

@okurz
Copy link
Member Author

okurz commented Jan 11, 2017

this comes from wait_screen_change. I can reference the documentation block from there, though, or just repeat the example in a short way without duplicating everything

@coolo
Copy link
Contributor

coolo commented Jan 11, 2017

Linking will do, yes.

@okurz okurz force-pushed the feature/assert_screen_change branch from c082982 to bae1ebe Compare January 12, 2017 07:34
@okurz
Copy link
Member Author

okurz commented Jan 12, 2017

updated

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 49.798% when pulling bae1ebe on okurz:feature/assert_screen_change into 6936e7f on os-autoinst:master.

@@ -396,6 +397,24 @@ sub wait_screen_change(&@) {
return 0;
}

=head2 assert_screen_change

assert_screen_change { CODEREF [,$timeout] };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actual incorrect syntax because the CODEREF is the {} - and the optinal timeout is after it. The same error is in the wait_screen_change reference

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this better?

diff --git a/testapi.pm b/testapi.pm
index 25a4157..165a8aa 100755
--- a/testapi.pm
+++ b/testapi.pm
@@ -354,7 +354,7 @@ sub assert_and_dclick {
 
 =head2 wait_screen_change
 
-  wait_screen_change { CODEREF [,$timeout] };
+  wait_screen_change(CODEREF [,$timeout]);
 
 Wrapper around code that is supposed to change the screen.
 This is the opposite to C<wait_still_screen>. Make sure to put the commands to change the screen
@@ -399,7 +399,7 @@ sub wait_screen_change(&@) {
 
 =head2 assert_screen_change
 
-  assert_screen_change { CODEREF [,$timeout] };
+  assert_screen_change(CODEREF [,$timeout]);
 
 Run C<CODEREF> with C<wait_screen_change> but C<die> if screen did not change
 within timeout. Look into C<wait_screen_change> for details

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Variant of wait_screen_change but dying on no screen change.
@okurz okurz force-pushed the feature/assert_screen_change branch from bae1ebe to efc2234 Compare January 12, 2017 09:19
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 49.798% when pulling efc2234 on okurz:feature/assert_screen_change into 6936e7f on os-autoinst:master.

@coolo coolo merged commit 3d5f1f5 into os-autoinst:master Jan 12, 2017
@okurz okurz deleted the feature/assert_screen_change branch January 12, 2017 21:28
okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Jan 13, 2017
os-autoinst/os-autoinst#689 added a new function to
the testapi 'assert_screen_change' which should be used wherever there is no
explicit checking of the return value of 'wait_screen_change'.

Leaving alone the occurences of 'wait_screen_change' where the return code
is checked.
okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Jan 13, 2017
os-autoinst/os-autoinst#689 added a new function to
the testapi 'assert_screen_change' which should be used wherever there is no
explicit checking of the return value of 'wait_screen_change'.

Leaving alone the occurences of 'wait_screen_change' where the return code
is checked.

Verification run: http://lord.arch/tests/5552
=cut

sub assert_screen_change(&@) {
::wait_screen_change(@_) or die 'assert_screen_change failed to detect a screen change';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know perl, what does :: do here? I just want to call the other method in the same file and without that it didn't work. Also, deleting the parameter part (&@) on both wait_screen_change and assert_screen_change did not work, ideas?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perl black magic, ::wait_screen_change is short for main::wait_screen_change

use strict;
use warnings;

sub foo {
    print "They called me!";
}

::foo;
foo;

okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Apr 20, 2017
os-autoinst/os-autoinst#689 added a new function to
the testapi 'assert_screen_change' which should be used wherever there is no
explicit checking of the return value of 'wait_screen_change'.

Leaving alone the occurences of 'wait_screen_change' where the return code
is checked.
okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request May 15, 2017
os-autoinst/os-autoinst#689 added a new function to
the testapi 'assert_screen_change' which should be used wherever there is no
explicit checking of the return value of 'wait_screen_change'.

Leaving alone the occurences of 'wait_screen_change' where the return code
is checked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants