-
Notifications
You must be signed in to change notification settings - Fork 266
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
Ensure to show development versions in registration #6556
Ensure to show development versions in registration #6556
Conversation
lib/registration.pm
Outdated
@@ -239,7 +239,7 @@ sub process_scc_register_addons { | |||
# we - Workstation | |||
# wsm - Web and Scripting Module | |||
if (get_var('SCC_ADDONS')) { | |||
if (check_screen('scc-beta-filter-checkbox', 5)) { | |||
if (check_screen('scc-beta-filter-checkbox', 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.
Could we make that an multi-tag assert_screen please to not rely on these unreliable timeouts?
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 would not know what to assert in that case to be honest, it is kind of how the worker reacts. Before we were waiting up to 30s so I think that should help and unblock the problem.
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'm here with @jknphy
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.
Well, probably we could just make that an assert_screen
based on product version and the BETA variable as well. I also understand that it might not be that easy to understand what to assert for in the alternative case. However, just bumping the timeout is a recipe for failure, just making the issue more sporadic and we all know how much we like these cases, right? ;) In https://openqa.suse.de/tests/2375348#step/scc_registration/10 we could look for an empty section below the header of "Available Extensions and Modules". For example see the old QAM SLE12 test https://openqa.suse.de/tests/1860127#step/scc_registration/10 not showing the checkbox. So just create a needle covering the header and the bigger checkbox for the first module to assert that there is no "beta filter checkbox" in between.
Actually I just looked and we already have the alternatives since 4201999 so I would really recommend to go with that.
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 am not sure it has an easy fix with a smart needle which will not break again the test in other product. It looks to me like the screen is not ready but there is not change in the screen from non-ready to ready, so the original timeout of 30s from check_screen was hiding this. I appreciate your input but I think we should merge it to unblock the whole column of the architecture, anyway we are not introducing any delay that was not already there (before I moved the pre-selected check) and we can think more deeply what needs to be done. WDYT?
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've noticed that we have scc-without-beta-filter-checkbox
which we could use, as these 2 needles cover all possible outcomes. As for alternative we could actually use wait_still_screen here to assure that UI is loaded and then it's safe to run check_screen
with 0 timeout.
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.
Would wait_still_screen
not be the same except that it wastes 7 seconds at least in all the cases? I would go for scc-without-beta-filter-checkbox
same as we do in the other step. This is what I meant with my suggestion above. Sorry if that was not clear.
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 taking a closer look to the code:
It fails here because now we don't execute right after verify_preselected_modules
(where there is 30s of waiting, which I believe to produce/help the shortcut to hit the UI). In turn we go through here immediately entering and sending alt-i again (as it is showed in the video that automation uncheck and check back again) when in this point in previous build should be just waiting and not going into that section.
Therefore I think it should help to introduce a waiting time with a needle after this first alt-i
, meaning checking that we really uncheck after sending the key.
Works without change: http://slindomansilla-vm.qa.suse.de/tests/1118 |
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.
LGTM
Thanks @SergioAtSUSE so the timeout can just help and come back to previous situation. |
b4310f7
to
4f971cd
Compare
@@ -388,6 +388,12 @@ sub process_scc_register { | |||
} | |||
} | |||
|
|||
sub show_development_versions { |
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.
\o/
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.
Looks much better now, thanks!
Do we need new needles for this one? |
Would you mind changing the PR description and subject line to not confuse people? I think you don't need to change the branch name ;P |
@rwx788 I am not sure about the needles, I hope not. I'm working with @SergioAtSUSE and @foursixnine to fix my share worker to provide verification. |
@jknphy actually, I just got an impression that "scc-beta-filter-unchecked" is a new tag, but it's not, so my question is answered, thanks! |
Ensure to show development versions in registration. When we removed the checking for the pre-selected modules we were removing also some waiting time which helps the shortcut to hit the UI in slower architectures like aarch64: https://openqa.suse.de/tests/2375348#step/scc_registration/12
(provided by @SergioAtSUSE below )