Run yast ui test with ncurse for pvm backend

[coolgw]

Run yast ui test with ncurse for pvm backend

• Related ticket: https://progress.opensuse.org/issues/110779
• Needles: na
• Verification run: https://openqa.suse.de/tests/10081723

[github-actions]

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

• Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files

[jknphy]

I think it doesn't make sense to do this hack each time before calling the function, could we consider to do it inside the function?

[jknphy]

I think it doesn't make sense to do this hack each time before calling the function, could we consider to do it inside the function?

@coolgw could be this code inside the method yast2_console_exec and not all the time outside or is there any troubles with that?
if it simplifies the code we could borrow this: https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/16020/files#diff-283b909b6e798ec5f92a947c75b4e9ef2cc6805d36e0cdcfadab2b8fbd0a80a0R155

[coolgw]

I think it doesn't make sense to do this hack each time before calling the function, could we consider to do it inside the function?

@coolgw could be this code inside the method yast2_console_exec and not all the time outside or is there any troubles with that? if it simplifies the code we could borrow this: https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/16020/files#diff-283b909b6e798ec5f92a947c75b4e9ef2cc6805d36e0cdcfadab2b8fbd0a80a0R155

yast2_console_exec is common function call and it's original design contain a %args can be used for flag ui mode, i do not want yast2_console_exec function contain a hidden hard code logic such as put ncurse mod if is_pvm. I think my implementation has more flexibility and fit original design, the special logic should put on test level not within function level, function level logic should base on parameter.
@ge0r @rakoenig what's your opinion ?

[jknphy]

I think it doesn't make sense to do this hack each time before calling the function, could we consider to do it inside the function?

@coolgw could be this code inside the method yast2_console_exec and not all the time outside or is there any troubles with that? if it simplifies the code we could borrow this: https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/16020/files#diff-283b909b6e798ec5f92a947c75b4e9ef2cc6805d36e0cdcfadab2b8fbd0a80a0R155

yast2_console_exec is common function call and it's original design contain a %args can be used for flag ui mode, i do not want yast2_console_exec function contain a hidden hard code logic such as put ncurse mod if is_pvm. I think my implementation has more flexibility and fit original design, the special logic should put on test level not within function level, function level logic should base on parameter. @ge0r @rakoenig what's your opinion ?

it is a hack, because we cannot test PowerVM in a different way, so single point of hack it is better than a hack all over the place, and if someone will face this in the future, needs to put this code again or ask. What I'm not sure if is_pvm would be enough, we need to check if we put inside that is the pvm_hmc one, because for spvm backend it might be possible to run some desktop stuff, I don't know that, we barely use it, only Kernel squad, I didn't see any desktop in the installed system.
Another point is that the fact that the method has some parameters, doesn't mean that you can give a default ones given some special conditions, like it is this case. For the name of the function you always expect ncurses because you are running in the console, but unless you want to do ssh-x in PowerVM you should set a proper default.

[coolgw]

I think it doesn't make sense to do this hack each time before calling the function, could we consider to do it inside the function?

@coolgw could be this code inside the method yast2_console_exec and not all the time outside or is there any troubles with that? if it simplifies the code we could borrow this: https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/16020/files#diff-283b909b6e798ec5f92a947c75b4e9ef2cc6805d36e0cdcfadab2b8fbd0a80a0R155

yast2_console_exec is common function call and it's original design contain a %args can be used for flag ui mode, i do not want yast2_console_exec function contain a hidden hard code logic such as put ncurse mod if is_pvm. I think my implementation has more flexibility and fit original design, the special logic should put on test level not within function level, function level logic should base on parameter. @ge0r @rakoenig what's your opinion ?

it is a hack, because we cannot test PowerVM in a different way, so single point of hack it is better than a hack all over the place, and if someone will face this in the future, needs to put this code again or ask. What I'm not sure if is_pvm would be enough, we need to check if we put inside that is the pvm_hmc one, because for spvm backend it might be possible to run some desktop stuff, I don't know that, we barely use it, only Kernel squad, I didn't see any desktop in the installed system. Another point is that the fact that the method has some parameters, doesn't mean that you can give a default ones given some special conditions, like it is this case. For the name of the function you always expect ncurses because you are running in the console, but unless you want to do ssh-x in PowerVM you should set a proper default.

I do not think add extra parameter for yast2_console_exec is a hack, it's normal way to extend function and make it start support more feature, so no hack all over the place, it should give correct parameter for function if needed.
In future if you need add more backend or change backend then you need modify on test module level, not function level, the low api test function level should touch as less as possible. I do not like some hidden logic under the function api, and add condition is_xxx is very popular in os-autoinst-distri-opensuse test module level.
Further more, in function level i do not add any default one or give some special conditions, default logic is as same as before and new logic ONLY happen if you add new parameter, i do not call that special conditions, it's new logic support for new parameter.

[jknphy]

I think it doesn't make sense to do this hack each time before calling the function, could we consider to do it inside the function?

@coolgw could be this code inside the method yast2_console_exec and not all the time outside or is there any troubles with that? if it simplifies the code we could borrow this: https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/16020/files#diff-283b909b6e798ec5f92a947c75b4e9ef2cc6805d36e0cdcfadab2b8fbd0a80a0R155

yast2_console_exec is common function call and it's original design contain a %args can be used for flag ui mode, i do not want yast2_console_exec function contain a hidden hard code logic such as put ncurse mod if is_pvm. I think my implementation has more flexibility and fit original design, the special logic should put on test level not within function level, function level logic should base on parameter. @ge0r @rakoenig what's your opinion ?

it is a hack, because we cannot test PowerVM in a different way, so single point of hack it is better than a hack all over the place, and if someone will face this in the future, needs to put this code again or ask. What I'm not sure if is_pvm would be enough, we need to check if we put inside that is the pvm_hmc one, because for spvm backend it might be possible to run some desktop stuff, I don't know that, we barely use it, only Kernel squad, I didn't see any desktop in the installed system. Another point is that the fact that the method has some parameters, doesn't mean that you can give a default ones given some special conditions, like it is this case. For the name of the function you always expect ncurses because you are running in the console, but unless you want to do ssh-x in PowerVM you should set a proper default.

I do not think add extra parameter for yast2_console_exec is a hack, it's normal way to extend function and make it start support more feature, so no hack all over the place, it should give correct parameter for function if needed. In future if you need add more backend or change backend then you need modify on test module level, not function level, the low api test function level should touch as less as possible. I do not like some hidden logic under the function api, and add condition is_xxx is very popular in os-autoinst-distri-opensuse test module level. Further more, in function level i do not add any default one or give some special conditions, default logic is as same as before and new logic ONLY happen if you add new parameter, i do not call that special conditions, it's new logic support for new parameter.

I'm referring that is a hack the fact that we need to adapt the test with that condition, because that env actually has desktop but we don't test it like that, so it is somehow our workaround. I'm fine if you prefer to do it like that, anyway these are legacy methods.