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

Handle java license popup during installation #16830

Merged
merged 1 commit into from Apr 12, 2023

Conversation

sofiasyria
Copy link
Contributor

@sofiasyria sofiasyria commented Apr 12, 2023

For SP4 there is an extra java license popup that breaks installation tests. This PR is adding a way to handle it in confirm_installation.pm

@sofiasyria sofiasyria added qe-yam WIP Work in progress labels Apr 12, 2023
@sofiasyria sofiasyria force-pushed the java_popup branch 3 times, most recently from df08231 to 6371edc Compare April 12, 2023 07:51
@sofiasyria sofiasyria removed the WIP Work in progress label Apr 12, 2023
tests/installation/confirm_installation.pm Outdated Show resolved Hide resolved
if ((get_var("SCC_ADDONS") =~ /legacy/) && (is_sle("=15-SP4"))) {
my $package_license_popup = $testapi::distri->get_accept_popup_controller();
$package_license_popup->wait_accept_popup({
timeout => 3000,
Copy link
Contributor

Choose a reason for hiding this comment

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

the confirm package popup happens intermediately, there is not need to have an explicit wait.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tried it already without the wait and it failed. It doesn't appear immediately enough

Copy link
Contributor

@jknphy jknphy Apr 12, 2023

Choose a reason for hiding this comment

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

can you share the link? what is the timeout and how much needs in the slower arch? 3000 is too much, and we should not abuse of this mechanism of explicit wait. We even have another ticket partially for that reason, https://progress.opensuse.org/issues/125561 to not repeat that code over and over, but still is a corner-case-code.

Copy link
Contributor

Choose a reason for hiding this comment

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

btw your runs are done in your machine, for timeout I would recommend OSD, we might find something funny...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am running some VRs on OSD now. For the failed runs without the timeout , it might be this one: http://falafel.suse.cz/tests/901#step/confirm_package_license/2 or it has disappeared.

Copy link
Contributor

Choose a reason for hiding this comment

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

that is some other failure during your development, I don't think is needed extra wait, we already wait 2 mins iirc,
Can't call method "exist" on an undefined value

Copy link
Contributor

Choose a reason for hiding this comment

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

ok you added to the ticket (just noticed), so I'm fine if we do in the follow-up.


sub run {
# For SLE 15 SP4 tests that have the Legacy module added during instalaltion, there is
# additional licence popup that is handled by the following function.
if ((get_var("SCC_ADDONS") =~ /legacy/) && (is_sle("=15-SP4"))) {
Copy link
Contributor

@jknphy jknphy Apr 12, 2023

Choose a reason for hiding this comment

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

we need a ticket to remove this condition:
the product part of the condition via our distribution pattern and the first part just passing as parameter to a future confirm_installation() method as we discussed in Slack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will open a ticket for this, but can't say that I have full understanding of your solution proposal. You want this java popup to be handled inside the confirm_installation() which will receive some arguments and act accordingly? The conditions should be inside the library?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please, check here: https://progress.opensuse.org/issues/127529

Copy link
Contributor

Choose a reason for hiding this comment

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

I rephrased a bit but you got the idea, thanks!

tests/installation/confirm_installation.pm Show resolved Hide resolved
@jknphy jknphy merged commit 13631ec into os-autoinst:master Apr 12, 2023
7 checks passed
@jknphy
Copy link
Contributor

jknphy commented Apr 13, 2023

@sofiasyria we probably needs to add to the condition the case where we have minimal patterns:
PATTERNS: base,minimal
https://openqa.suse.de/tests/10910263#step/confirm_installation/1

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