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

Get rid of wait_still_screen calls #6792

Merged
merged 1 commit into from Mar 7, 2019

Conversation

@rwx788
Copy link
Member

rwx788 commented Feb 18, 2019

I've added notready label, so we don't merge it accidentally.

@mloviska
Copy link
Contributor Author

os-autoinst/os-autoinst#1106 got merged!

@okurz
Copy link
Member

okurz commented Feb 25, 2019

both needles PR merged


my $self = shift;
my $module = "host";
my $dm = lc get_var('DESKTOP');
Copy link
Member

Choose a reason for hiding this comment

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

care to use get_required_var instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, I have forgotten about this subroutine. Ty!

@mloviska mloviska added Ready Ready for review and removed notready labels Feb 27, 2019
send_key 'tab';
type_string($hosts_params->{fqdn}, max_interval => 13, wait_still_screen => 0.05, timeout => 5, similarity_level => 38);
send_key 'tab';
type_string($hosts_params->{alias}, max_interval => 13, wait_still_screen => 0.05, timeout => 5, similarity_level => 38);
Copy link
Member

Choose a reason for hiding this comment

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

What's about having some wrapper with given set of parameters, as I guess they were derived empirically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean to put all empirical values under one data structure? Or do you suggest to somehow wrap them into a subroutine?

Copy link
Member

Choose a reason for hiding this comment

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

I guess more like second, so similarly to type_string_slow and type_string_very_slow. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

So, to resolve this one, what do you think about some wrapper? If you object, I'm fine to leave it as is, but I'm quite sure that this will be handy in many other scenarios.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay. I will deliver the update during the day.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

are you ok to leave the wrapper for now in the test module code? I am not sure if I have set the values correctly for OSD or O3. I am expecting some minor updates after the code gets merged.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, fine with me. Let's merge then ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, I have pushed the updates a few minutes after the PR got merged. ups...

Copy link
Member

Choose a reason for hiding this comment

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

I've misunderstood you. But no worries, create new PR ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rwx788
Copy link
Member

rwx788 commented Mar 4, 2019

os-autoinst changes were deployed, so we can proceed with this one

@rwx788 rwx788 merged commit c18047c into os-autoinst:master Mar 7, 2019
@DimStar77
Copy link
Contributor

DimStar77 commented Mar 7, 2019

Breaks various gnome tests, like: https://openqa.opensuse.org/tests/873148#

the same is observed in stagings, out of stagings, on Leap and on Tumbleweed (@nilxam - this should be the same for your leap issues)

@rwx788
Copy link
Member

rwx788 commented Mar 7, 2019

@mloviska is not available on irc, so I will revert and retrigger the jobs which I will be able to find

@DimStar77
Copy link
Contributor

Propsal fix - without having to revert all of it: #6987

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready Ready for review
Projects
None yet
5 participants