Wait for Installation Settings screen to be loaded #13990
Conversation
| @@ -14,6 +14,7 @@ use Test::Assert 'assert_true'; | |||
|
|
|||
| sub run { | |||
| my $installation_settings = $testapi::distri->get_installation_settings(); | |||
| $installation_settings->wait_for_overview_content_to_be_loaded(); | |||
There was a problem hiding this comment.
what about to run this method after the action that we know cause a refreshment, so basically we could do it internally after changing the service or the port, the test doesn't know about refreshments let's say, but we can go with this if you want it is a good workaround without having the solution in server side, wdyt?
There was a problem hiding this comment.
But in this case we don't do changes that cause the refreshment on Installation Setting. It's just initial page load when navigating from previous screen to Installation Settings.
There was a problem hiding this comment.
any reason why not to put it in get_overview_content ? At least I would have put it there :) but yeah, just to gathering feedback from you as I'm not sure about this and I had the same situation in my previous PRs. We can merge either way.
There was a problem hiding this comment.
The reason is that if we will need to create a test that checks, that the page has placeholders when it is loading, we'll not be able to do so. For sure that may never be the case, but we'll keep single responsibility principle which is also keep our solution more consistent.
There was a problem hiding this comment.
ahm I was referring to is_shown() which means that if the screen is loaded, I guess that method should be more complicated in this particular case, so we can break the single principle because we don't have the proper server solution. For the rest of screen we don't need to do that because there is not refreshment. With this approach we always have to add this new method to test (which we are not supposed to change much). Does it convince you? :)
validate_ssh_service_enabled test module checked that Installation Screen is loaded by ensuring that widget with "id=proposal" is shown on the screen. Then 'assert_true' is called immediately and it failed, because the content of 'proposal' widget was not loaded. The commit ensures that the the content is loaded before validating that the port is opened. Related issue: https://progress.opensuse.org/issues/104703
OleksandrOrlov
force-pushed
the
104703_wait_for_inst_settings_load
branch
from
2eede62
to
f5ac701
Compare
| @@ -29,9 +29,18 @@ sub init { | |||
| sub get_installation_settings_page { | |||
| my ($self) = @_; | |||
| die "Installation Settings Page is not displayed" unless $self->{InstallationSettingsPage}->is_shown(); | |||
| die "Overview content on Installation Settings Page is not loaded completely" unless $self->{InstallationSettingsPage}->is_loaded_completely(); | |||
There was a problem hiding this comment.
Can we merge these two lines? are not the same thing? is_shown, is_loaded are the same to me, but check my comment below.
| return $self->{InstallationSettingsPage}; | ||
| } | ||
|
|
||
| sub wait_for_overview_content_to_be_loaded { |
There was a problem hiding this comment.
is this still in use, or you forgot to delete it?
| sub is_loaded_completely { | ||
| my ($self) = @_; | ||
| my $result; | ||
| eval { |
There was a problem hiding this comment.
so you silent the method with the eval I think this was a no-go, only should be used in the client itself, other contributors claimed it was bad practice in Perl, it doesn't behave so well as try/catch in other language (some modern languages even avoid try/catch as a build-in feature of the language).
I think it would be easier just to use the is_shown and add to wait_until a non-failure feature so we do the hack at the inner level, the only problem is the error if we die or not to use it for what I can see ... wdyt?
|
Given that #14022 is still not done, in order to not block it for more time, let's merge it. Anyway the approach with the |
validate_ssh_service_enabled test module checked that Installation
Screen is loaded by ensuring that widget with "id=proposal" is shown on
the screen. Then 'assert_true' is called immediately and it failed,
because the content of 'proposal' widget was not loaded.
The commit ensures that the the content is loaded before validating that
the port is opened.