Improve user typing workaround #9780
Conversation
SergioAtSUSE
force-pushed
the
improve_user_typing_workaround
branch
from
aaae05a
to
1b4ac70
Compare
lib/installation_user_settings.pm
Outdated
| wait_screen_change { | ||
| if (defined $args{max_interval}) { | ||
| type_string($args{username}, max_interval => $args{max_interval}); | ||
| } else { | ||
| type_string($args{username}); | ||
| } |
There was a problem hiding this comment.
I think you can do something like this
| wait_screen_change { | |
| if (defined $args{max_interval}) { | |
| type_string($args{username}, max_interval => $args{max_interval}); | |
| } else { | |
| type_string($args{username}); | |
| } | |
| $args{max_interval} //= undef; | |
| ... | |
| wait_screen_change { | |
| type_string($args{username}, max_interval => $args{max_interval}); |
Since the definition of type_string in testapi sets it to 250 if it's not defined
There was a problem hiding this comment.
I wanted to be explicit. I prefer to not send undef. If there is no parameter to send, better send no parameter at all.
But, if it is an issue for you I can change it.
There was a problem hiding this comment.
+1 to the @foursixnine suggestion
tests/installation/user_settings.pm
Outdated
| if ($retry == 0) { | ||
| $self->enter_userinfo(); | ||
| } else { | ||
| $self->enter_userinfo(max_interval => utils::VERY_SLOW_TYPING_SPEED); |
There was a problem hiding this comment.
I'd go for something like this, but the inner me says: ]Explicit is better than implicit](https://www.python.org/dev/peps/pep-0020/)
| $self->enter_userinfo(max_interval => utils::VERY_SLOW_TYPING_SPEED); | |
| $self->enter_userinfo(max_interval => ($retry)? utils::VERY_SLOW_TYPING_SPEED : undef ); |
There was a problem hiding this comment.
Exactly. there is no gain in doing a one-liner.
There was a problem hiding this comment.
hey, this is perl, meaning … probably we would need to follow the opposite ;P
There was a problem hiding this comment.
More than one way to do it. I choose python way.
SergioAtSUSE
force-pushed
the
improve_user_typing_workaround
branch
from
1b4ac70
to
28b8aaa
Compare
Fail if workaround is not successful. Set typing speed by caller.
SergioAtSUSE
force-pushed
the
improve_user_typing_workaround
branch
from
28b8aaa
to
8806de7
Compare
|
@SergioAtSUSE Our tests start fails after this PR merged, any idea? Is this a side effect of this PR? |
Also affecting x86 jobs: https://openqa.nue.suse.com/tests/3998970#step/user_settings/11 |
They didn't start failing, they were failing but shown as orange. The last good was failing: https://openqa.suse.de/tests/3982502#step/user_settings/10 but shown orange. Then any dependent image can fail later. Please try increasing the retries or decreasing the max_interval to make the workaround to work. |
yes, x86_64 is also hit by typing issues. But, this PR is not making the workaround to not work, it is making it to not ignore a mistype. If there is a mistype after applying the workaround, then the workaround didn't work and the test shall fail. |
|
This is really a good idea. When people find their job task turns into red, they probably manually retrigger those task again and again to green, then all orange turns into green. Fantastic ;) |
|
no offense, I really don't think remove orange is a good idea. If people what to care softfail, just mandatory review those softfail. |
Now, because there are more teams annoyed by failing test that they don't care than a team "functional" that cares, I will find a way of only fail on functional job group. |
Our job group had more than 20 soft-fails in module user_settings: https://openqa.suse.de/tests/overview?arch=&machine=&modules=user_settings&modules_result=softfailed&distri=sle&version=15-SP2&build=154.1&groupid=110# Some of then were soft-fail with working workaround, some of then where soft-fail with not working workaround. That means that to "resolve the typing issue" you would need to look into all those and find those which are mistyping after the workaround. Now, we difference between the working workaround (soft-fail) and not working workaround (fail). But, as I have already said, I agree with your pain and I am looking for a way to make that tests outside functional ignore again the mistype ;) |
|
@SergioAtSUSE thank you very much for your help. I understand your problem now. It's really hard to balance these situations. |
After merge of os-autoinst#9780 we start fail test every time there is typo in username This looks irrelevant for HPC because we not using full username anywhere. So we okay if there is typo in username
After merge of os-autoinst#9780 we start fail test every time there is typo in username This looks irrelevant for HPC because we not using full username anywhere. So we okay if there is typo in username
Fail if workaround is not successful.
Set typing speed by caller.