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

wicked: add ifreload tests #11624

Merged
merged 1 commit into from Dec 18, 2020
Merged

Conversation

asmorodskyi
Copy link
Member

This patch bring coverage for functionality introduced in wicked 0.6.64
so prior this version this tests will fail

This patch bring coverage for functionality introduced in wicked 0.6.64
so prior this version this tests will fail
@asmorodskyi
Copy link
Member Author

@cfconrad this is your code actually :) but please double check anyway

@asmorodskyi asmorodskyi removed the WIP Work in progress label Dec 16, 2020
@asmorodskyi
Copy link
Member Author

@jlausuch
Copy link
Contributor

I guess the reason why executing a bash script instead of writing the code in perl logic is due to high amount of bash commands, am I right? How have you come up with those bash scripts? Are they taken from developers?

Are those 4 new test cases any of the missing in the startstop legacy test suite?

@asmorodskyi
Copy link
Member Author

I guess the reason why executing a bash script instead of writing the code in perl logic is due to high amount of bash commands, am I right? How have you come up with those bash scripts? Are they taken from developers?

scripts are from developers ( Marius wrote them ) and this is actually main reason why it is in bash ( obviously he don't want to mess up with perl test code )

Are those 4 new test cases any of the missing in the startstop legacy test suite?

none of this test cases presented in legacy test suite because they covering functionality introduced in 0.6.64 ( latest for today ) version of wicked

@mloviska
Copy link
Contributor

Off topic (sorry!)

Martin has created a nice shell runner for openQA. I would like to share it with you as there so many scripts, thus it might be useful in the future.

#9219

@asmorodskyi
Copy link
Member Author

Off topic (sorry!)

Martin has created a nice shell runner for openQA. I would like to share it with you as there so many scripts, thus it might be useful in the future.

#9219

thanks for the hint ! I really like this idea ! and I think it will be right thing to move this scripts to use this shell runner. On the other side I notice that to use this approach you need to modify existing bash scripts to met some requirements while this PR already contain fully workable approach already used by wicked developers for some time in their internal openQA instance . So I would prefer to merge this PR as it is and come up with another one to refactor /test approach which you suggesting

@mloviska
Copy link
Contributor

thanks for the hint ! I really like this idea ! and I think it will be right thing to move this scripts to use this shell runner. On the other side I notice that to use this approach you need to modify existing bash scripts to met some requirements while this PR already contain fully workable approach already used by wicked developers for some time in their internal openQA instance . So I would prefer to merge this PR as it is and come up with another one to refactor /test approach which you suggesting

I haven't checked the details, yet. Thanks for going deeper! Yup, it was just an idea whether we can use it. Eventually, it might not be so much helpful for us. Let's see in the future :)

@asmorodskyi asmorodskyi merged commit 91fbf9b into os-autoinst:master Dec 18, 2020
@cfconrad
Copy link
Contributor

cfconrad commented Jan 5, 2021

@asmorodskyi thx for moving the tests to osd.

Regarding shell runner, I don't see the benefit of converting the scripts, I would avoid dependencies when just running one script. The cool thing is, that they split stdout and stderr, but if this is really needed, we can add a redirect in our version too.

@asmorodskyi asmorodskyi deleted the ifconfig branch February 12, 2021 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants