Basic functional test

[dcermak]

This is my current state of the openQA tests for the kiwi staging area.
This fixes #2, #5, #6

What it does:

• iso images are booted, login is tested and either the vm is turned off, if this is an installation iso or rebooted and reloged in, if it is a live iso
• disk images are booted, user is logged in, rebooted, user logs in again and then the VM is turned off
• the created hard drive of an installation iso is saved and goes through the same test as any other kiwi disk image

what currently does not work:

• login on the openstack qcow2 image is broken, the default credentials do not work (fixed by @schaefi, the image had its root console deactivated)
• all the raw disk images appear not to work, for some reason openQA cannot create a working disk image from these
• kiwi-test-image-disk-legacy.x86_64.iso and kiwi-test-image-custom-partitions.x86_64.iso hang during the installation and never reach a login prompt
• the hackery that I added to select the correct entry in the boot menu is not really that great and does only appear to work by accident

What still needs to be done:

• documentation what is going on and how to configure openqa (external settings are necessary)
• [ ] add some actually meaningful testing, currently we just test whether this image boots
• test other distributions as well
• reconsider whether we should include the build id of the images in the schedule command: calling them by their static link means that openQA does not download newer versions if the old asset stays around (this should be done)

[schaefi]

@dcermak I added the "do not merge" label since you are still working on it. Please drop the label when you are done. Thanks

[okurz]

These both assert_screen … check_screen calls can likely be combined
with a multi-tag assert_screen with match_has_tag instead of check_screen with non-zero timeout to prevent introducing any timing dependant behaviour, to save test execution time as well as state more explicitly from the testers point of view what are the expected alternatives. A common simple example that I supply should explain it:

assert_screen([qw(yast2_console-finished yast2_missing_package)]);
if (match_has_tag('yast2_missing_package')) {
    send_key 'alt-o';  # confirm package installation
    assert_screen 'yast2_console-finished';
}

so let me try to suggest what you could here, e.g. like this:

assert_screen [qw(kiwi_bootloader kiwi_bootloader_boot_from_hdd)];
if match_has_tag 'kiwi_bootloader_boot_from_hdd' && defined(get_var("PUBLISH_HDD_1")) {
…

[okurz]

While using wait_screen_change and wait_still_screen sound simpler in some cases they often either waste time or introduce timing behaviour dependant problems. wait_screen_change means that this line finishes as soon as any screen change is seen, which might before the system is ready to accept the next line where you type the password.

It is often better to be explicit and have a proper needle check in the middle:

assert_screen 'login_prompt';
type_string 'root';
send_key 'ret';
assert_screen 'password_prompt';
type_string 'linux';
send_key 'ret';
assert_screen 'textmode_logged_in';

I see that you have added code further down to handle "grub stuff on the screen". Likely your needle for 'textmode_logged_in' is too big and hence too restrictive as it might fail to be detected due to the "grub stuff". If that is the case think like a human. What do you really look for on the screen to detect that the "textmode is logged in"? Likely it is just something like the prompt sign and some free space to the right of it, e.g. # and nothing else :) This is what is included in the corresponding needle within os-autoinst-distri-opensuse

[okurz]

just a matter of taste. I would write no parentheses for such simple code and use single ticks to prevent accidental variable expansion where not explicitly desired, e.g.:

    assert_screen 'textmode_logged_in';
    assert_script_run 'reboot';
    assert_screen 'kiwi_bootloader';
    assert_screen 'login_prompt';

[dcermak]

The single tick backticks are a good idea, thank you!

But unless @schaefi insists on leaving out the parenthesis, I'd really like to keep them (I just don't like this shell like parameter passing convention in perl).

[schaefi]

I agree with you on the parenthesis. As I come from python and formerly from C and also wrote perl in the C style syntax I'm used to parenthesis for function parameter lists. Nevertheless I have no super strong opinion about it if we do it consistently. So parenthesis everywhere or nowhere but no mix please :)

I also checked on perlcritic in --brutal mode and it does not complain if you use parenthesis or not. On the other side it does complain if you use builtin methods like e.g print with parenthesis. This means if you want to handle this topic in a consistent way and perlcritic --brutal clean code you are forced to prevent parenthesis unless a custom .perlcritic file configures the desired behavior.

In the end this is personal taste and I only want consistency if that makes sense to you too

[okurz]

If this is only temporarily shown, e.g. due to an automatic timeout I recommend to not have that check at all due to the possibility that the test system might "miss" the screen and then fail when the SUT already continues booting. This happened in other cases hence I am sharing that problem from the past. A good hint how to design openQA tests in this regard: Think about how a manual tester would type in "reboot" and then goes away to get a cup of coffee. After return the test would see the 'login_prompt' only, not the intermediate state. If you ask the overly stupid tester to "check for kiwi_bootloader" that tester would say the test failed as the login prompt is shown instead of the expected "kiwi_bootloader" screen :)

By the way, openQA automatically records screenshots if the screen changes more than a defined threshold so it's likely that the bootloader screen would show up as intermediate screenshot anyway regardless of an explicit check or not.

[dcermak]

Hm, this makes sense, but then again, not checking for the bootloader could also result in the situation that you just press ctrl+d and never reboot. The test would then still pass, as it only checks that the login screen is present and newer for the bootloader. But not sure whether it really makes sense to worry about that.

[okurz]

If you want to be explicit then disable the grub timeout before reboot, trigger reboot, wait for grub menu, and explicitly call send_key 'ret'.

Basically I see three options:

1. leave it as is
2. Add the explicit grub menu check with disabled timeout and explicit boot confirmation action needed
3. reboot, wait for login and check afterwards that a reboot actually happened, e.g. in logs or uptime
4. just remove the check and not know if the system actually rebooted

I am just providing the options and of course I am in no position to "block" your PR for this repo here :) But based on my experience I just advise against 1. and invest the effort to go for 2., 3. or 4. because I am convinced that sooner or later 1. will cause more pain and time wasted to understand what is going on than what one would consider worth the effort :)

[okurz]

@dcermak as over the past months you have updated this PR but it does not look like you update to have it merged I will unsubscribe from the PR to not get repeated notifications. Feel free to ping me again as soon as you updated the PR for actual review.

[dcermak]

@okurz we have decided during this week's kiwi community meeting that we are going to drop this PR and just merge the whole thing via #10 as soon as the tests succeed on my local box so that others can collaborate and help get them to run on o3.