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

Use libyui api for yast2 lan dhcp hostname setting #12127

Merged
merged 2 commits into from Mar 19, 2021

Conversation

OleksandrOrlov
Copy link
Contributor

@OleksandrOrlov OleksandrOrlov commented Mar 12, 2021

The commit rewrites yast2_lan_hostname tests to use libyui-rest-api.

Also the test cases are separated for the cases when 'no', 'yes: any'
and 'yes: eth0' are set for hostname via DHCP.

The following update is required on o3 in Job Groups:

  • openSUSE Tumbleweed
  • openSUSE Tumbleweed AArch64
  • openSUSE Tumbleweed PowerPC
  • openSUSE Leap 15
- yast2_ncurses:
    settings:
       YAML_SCHEDULE: schedule/yast/yast2_ncurses/yast2_ncurses_opensuse.yaml

lib/YaST/Module.pm Outdated Show resolved Hide resolved
lib/YaST/Module.pm Outdated Show resolved Hide resolved
Copy link
Contributor

@jknphy jknphy left a comment

Choose a reason for hiding this comment

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

In general, I would recommend to improve the test data in this PR.

Other points:
Besides that, s/libyu/libyui/ in the title and in the commit and as discussed offline, separate the commits.
Also, in order to not forget, we have multiple implementation of the notification dialog, we should put a comment or make it more explicit that is related to use NM, should be obvious somehow from the test data/test what we are doing different there (different opening methods? just thinking loud, like open_lan_nm, open_lan, I know that that is not how you implement it, just to consider it)

lib/YaST/Module.pm Show resolved Hide resolved
lib/YaST/NetworkSettings/ActionButtons.pm Outdated Show resolved Hide resolved
lib/YaST/NetworkSettings/HostnameDNSTab.pm Outdated Show resolved Hide resolved
lib/YaST/Warning/Notification.pm Show resolved Hide resolved
schedule/yast/yast2_ncurses/yast2_ncurses_opensuse.yaml Outdated Show resolved Hide resolved
tests/console/yast2_lan_hostname_dhcp_yes_eth.pm Outdated Show resolved Hide resolved
tests/console/yast2_lan_hostname_dhcp_yes_eth.pm Outdated Show resolved Hide resolved
});
$network_settings->save_changes();

assert_screen 'console-visible', 180;
Copy link
Contributor

Choose a reason for hiding this comment

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

why we should still use needles, can we just change console here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially I had select_console 'root' here. By the way, it also uses needles inside:)

But I had some issues with waiting and thought it is due to that. So, I returned the approach that was used in the initial yast2_lan_hostname test module. The issues were not gone:) So I can revert to select_console 'root', but I have better approach here, that I want to apply.

I want to create something like wait_for_close in YaST::Module and use wait_serial inside to check that module is closed.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we want to promote this method and extract to some method? No need to do it now though.

tests/console/yast2_lan_hostname_dhcp_no.pm Outdated Show resolved Hide resolved

assert_screen 'console-visible', 180;
wait_still_screen;
assert_script_run 'grep DHCLIENT_SET_HOSTNAME /etc/sysconfig/network/dhcp|grep no';
Copy link
Contributor

Choose a reason for hiding this comment

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

btw we are already validating settings with https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/lib/cfg_files_utils.pm, there is even some validation of network #11954

@OleksandrOrlov OleksandrOrlov force-pushed the 89209_libyui_lan2_hostname branch 2 times, most recently from db48571 to 7a646cd Compare March 15, 2021 15:28
@rwx788 rwx788 changed the title Use libyu api for yast2 lan dhcp hostname setting Use libyui api for yast2 lan dhcp hostname setting Mar 15, 2021
@OleksandrOrlov OleksandrOrlov force-pushed the 89209_libyui_lan2_hostname branch 3 times, most recently from 280dff0 to 0cb864e Compare March 16, 2021 10:08
@OleksandrOrlov OleksandrOrlov force-pushed the 89209_libyui_lan2_hostname branch 5 times, most recently from 64dead3 to 6c96cfb Compare March 17, 2021 14:26
);
}
elsif ($ui eq 'qt') {
launch_yast2_module_x11('', $module, extra_vars => get_var('YUI_PARAMS'));
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 introduced PR where I removed $self from launch_yast2_module_x11. So after the PR is merged, here workaround for the 1st parameter also has to be removed.
PR: #12159

Copy link
Member

@rwx788 rwx788 left a comment

Choose a reason for hiding this comment

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

LGTM

@rwx788
Copy link
Member

rwx788 commented Mar 18, 2021

Before we merge your change, could you please take a look on this failure in your VRs: https://openqa.suse.de/tests/5680628#step/yast2_lan_restart_bridge/31 Might be related to always_rollback flag being set.

@OleksandrOrlov OleksandrOrlov force-pushed the 89209_libyui_lan2_hostname branch 2 times, most recently from 88dd3b2 to 1173d38 Compare March 18, 2021 11:21
@OleksandrOrlov OleksandrOrlov force-pushed the 89209_libyui_lan2_hostname branch 2 times, most recently from f0fef4c to 303f753 Compare March 18, 2021 11:35
@OleksandrOrlov
Copy link
Contributor Author

OleksandrOrlov commented Mar 18, 2021

Before we merge your change, could you please take a look on this failure in your VRs: https://openqa.suse.de/tests/5680628#step/yast2_lan_restart_bridge/31 Might be related to always_rollback flag being set.

Fixed. It was because yast2_lan_restart_* tests used save_changes method, but it was rewritten with LibyuiClient. So, I'm running the modules now with libyui-rest-api support and it works.

I also created a ticket to use LibyuiClient in the modules completely: https://progress.opensuse.org/issues/90287

Oleksandr Orlov added 2 commits March 19, 2021 09:34
The commit rewrites yast2_lan_hostname tests to use libyui-rest-api.

Also the test cases are separated for the cases when 'no', 'yes: any'
and 'yes: eth0' are set for hostname via DHCP.

Related ticket: https://progress.opensuse.org/issues/89209
Use open() method from YaST::Module to open the YaST module.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants