Note that those tests that were previously validated with f29 are now failing on f30 and Rawhide as reported in bug https://bugzilla.redhat.com/show_bug.cgi?id=1691832

Thanks for working on this!

So, some notes:

1. This only adds the tests to ppc64le arch - can you please add them for aarch64 and x86_64 too?
2. I think a much simpler implementation of install_repository_hd_variation should be possible. I think we can junk the install_repository_hd test entirely, then we can have install_repository_hd_variation first boot to rescue mode and dump the ISO onto the second disk, then reboot and run through the normal install steps. I might be missing something, but I think that ought to be viable.
3. I'm not 100% sure I'm in love with the use of dd to effectively recreate the ISO, though I guess it ought to work. An alternative is to simply re-download it from ISO_URL...
4. I'd prefer a cleaner approach than stuffing a ton of test modules into ENTRYPOINT. In my suggestion under 2), what I might suggest doing is add a sort of 'pre-install' step to main.pm - just before load_install_tests, have some kind of mechanism where we can specify additional modules to be loading before the install tests. So we'd just have to set the option to load one additional test module in that 'pre-install' phase for the install_repository_hd_variation test suite, then we could rely on the existing logic to load the rest of the install tests afterwards (of course our new test module would reboot at the end).

I'll add a few more trivial notes in-line. Thanks!

I'd rather you do this in get_full_repo, which already has some conditionalization: it only mungs the URL if it doesn't start with nfs. You could just tweak that so it also doesn't do any munging if the repo starts with hd.

Ditto to previous note for _boot_to_anaconda - instead of adding conditionalization there and here, just do it in get_full_repo, it's cleaner and we only have to do it once!

@adamwill my answers to your initial notes after my rebase of this PR.

Thanks for working on this!
So, some notes:

This only adds the tests to ppc64le arch - can you please add them for aarch64 and x86_64 too?

I did that in new commits, but did not tested them for those arches.

I think a much simpler implementation of install_repository_hd_variation should be possible. I think we can junk the install_repository_hd test entirely, then we can have install_repository_hd_variation first boot to rescue mode and dump the ISO onto the second disk, then reboot and run through the normal install steps. I might be missing something, but I think that ought to be viable.

I did not implemented such change, because I do not know how I could trigger the install after reboot in such a scenario, with no change of test parameter between the two phases.

I'm not 100% sure I'm in love with the use of dd to effectively recreate the ISO, though I guess it ought to work. An alternative is to simply re-download it from ISO_URL...

I did not re-download from ISO_URL because of impact on download time;
but I completed the dd usage by a checksum compare to be sure that same as iso from compose.

I'd prefer a cleaner approach than stuffing a ton of test modules into ENTRYPOINT.
In my suggestion under 2), what I might suggest doing is add a sort of 'pre-install' step to main.pm - just before load_install_tests, have some kind of mechanism where we can specify additional modules to be loading before the install tests. So we'd just have to set the option to load one additional test module in that 'pre-install' phase for the install_repository_hd_variation test suite, then we could rely on the existing logic to load the rest of the install tests afterwards (of course our new test module would reboot at the end).

In fact the ENTRYPOINT was useless for this test, I removed it and simply added INSTALL=1 as explained in templates.

I'll add a few more trivial notes in-line. Thanks!

I'd rather you do this in get_full_repo, which already has some conditionalization: it only mungs the URL if it doesn't start with nfs. You could just tweak that so it also doesn't do any munging if the repo starts with hd.

I'd rather you do this in get_full_repo, which already has some conditionalization: it only mungs the URL if it doesn't start with nfs. You could just tweak that so it also doesn't do any munging if the repo starts with hd.

Thanks, has been done in new rebase.

"I did not implemented such change, because I do not know how I could trigger the install after reboot in such a scenario, with no change of test parameter between the two phases."

With the strategy I described, I think this should 'just work'. If as I suggested we add a sort of 'pre-install' phase to the main.pm logic and load the test module that boots to rescue mode, does the necessary stuff, and then reboots to that phase, the install test modules should naturally load after that test and pick up after the reboot.

I guess the only question is whether the system would still boot from CD/DVD on that second boot...I hadn't entirely thought that bit through, but I'm not sure what the answer is.

Still, even if the answer turns out to be "it tries to boot from hard disk on the second boot and fails", and we can't figure out a way to work around that, so we still need a separate test - I still think it'd be simpler for the separate test to work by booting the installer image to rescue mode, rather than running after install_default_upload and using the disk image it uploads.

@adamw as suggested I implemented the use of a PREINSTALL to have the work done in one test; I validated that on a ppc64le. I did not tried on x86_64 or aarch64.

This can just be: if ($repourl !~ m/^(nfs|hd:)/) {

it'd probably be a good idea to make this conditional on being able to find ISO_URL and the CHECKSUM file, and not bail if one of those things isn't true. I can imagine circumstances where it won't be.

excellent write-only code here =)

please add a comment explaining this briefly, and also document PREINSTALL in the VARIABLES.md file (there are probably some other things I've forgotten to add there, I'm awful, but I am also a hypocrite so I'm going to make you do it :>)

looking at this, I really hate that it's duplicated with the rescue mode test. maybe just factor the whole thing into anacondatest or utils and have both tests use it from there?

did you try just using root_console here? I think it should work, thanks to needles/console/root_logged_in-noprofile-bash44-20170902.json.

did you try just using root_console here? I think it should work, thanks to needles/console/root_logged_in-noprofile-bash44-20170902.json.

That one is a bash shell, while rescue is a sh shell, so will not work.

it'd probably be a good idea to make this conditional on being able to find ISO_URL and the CHECKSUM file, and not bail if one of those things isn't true. I can imagine circumstances where it won't be.

Sorry my english is not good enough, are you expecting something here ?
What is the scenario you imagine ?

last rebase address last @adamwill remarks except for my two comments above.

@michelmno what I'm saying is I can imagine a situation where ISO_URL is not set or can no longer be accessed, or where it is set but there is no CHECKSUM file in the same directory. Those won't be the usual circumstances, but they may happen. I think it'd be best to set things up so that the test only fails if we can find the CHECKSUM file and the comparison actually fails; if we just don't manage to get the CHECKSUM file to compare against at all, I think it would be better to continue the test (possibly this could be a soft failure).

On the shell thing - ah, OK. I still might want to make this just another root_console needle, rather than a separate thing, but I'll maybe just fix that up myself on commit or with a follow-up commit.

Thanks again for working on this! I will test it out today.

So I've pushed this out to staging openQA, and it seems to be working...only both tests fail, I'm guessing due to 1691832 . The fix for that is due to go out to stable soon, so I think I'm going to wait for that and see how the tests behave with a compose that has the fix before merging this.

in last compose 20190412 the needle do not match because no presence of cursor that is part of needle reference that matched in previous run of 20190411.
https://openqa.stg.fedoraproject.org/tests/518128#step/preinstall_iso_in_hd/4 <= 20190412 only 83% matching as missing cursor.
https://openqa.stg.fedoraproject.org/tests/518128#step/preinstall_iso_in_hd/4 <= 20190411 100%

may be would need to increase timeout (more than 5s)

oh, just please tweak the needle not to include the cursor. That's never a good idea since it blinks. Please just follow the existing console needles...

I'm gonna merge this and clean up a couple of little issues with subsequent commits. Thanks!

Pagure rebase seems to be busted, so I merged this manually.