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

Unschedule yast2_lan_restart_devices when Network Manager #5780

Merged

Conversation

jknphy
Copy link
Contributor

@jknphy jknphy commented Sep 17, 2018

Missed my last commit in #5752 (probably I had some issues with rebase). This should unschedule the second part of the test as it doesn't make sense when Network Manager is default.

@@ -1438,7 +1438,7 @@ sub load_extra_tests_desktop {
# well
if (check_var('DESKTOP', 'gnome')) {
loadtest 'x11/yast2_lan_restart';
loadtest 'x11/yast2_lan_restart_devices';
loadtest 'x11/yast2_lan_restart_devices' unless (is_opensuse && !is_leap('<=15.0'));
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, seems that I miss something, but haven't you adjusted this test suite to work with network manager?
Also, multiple conditions with unless are not as readable as it's if version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was a time ago a big test and I split this scenario in two. In yast2_lan_restart we only check tabs as most of the thing are disabled. yast2_lan_restart_devices is testing some 'more advance' configuration, so the whole test does't make sense to schedule it.
Let me fix the condition, think is not so cumbersome, my mistake.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. Good approach, thanks for the clarification!

@jknphy jknphy force-pushed the network_manager_minor_fix_schedule branch from e09b7a6 to af6499c Compare September 17, 2018 11:35
@asmorodskyi asmorodskyi merged commit f2e574b into os-autoinst:master Sep 17, 2018
@jknphy
Copy link
Contributor Author

jknphy commented Sep 18, 2018

@jknphy
Copy link
Contributor Author

jknphy commented Sep 18, 2018

PR with the change: #5788

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