Factor meat out of advisory_post and do it in postfail too

If an update test fails before reaching advisory_post, we don't
generate the 'what update packages were installed' and 'were
any update packages *not* installed when they should have been'
logs, but these may well be useful for diagnosing the failure -
so let's also do the same stuff there. Only let's not do it all
twice.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

lib/installedtest.pm

@@ -78,6 +78,11 @@ sub post_fail_hook {
     upload_logs "/etc/nsswitch.conf", failok=>1;
 }
 
+    # For update tests, let's do the update package info log stuff,
+    # it may be useful for diagnosing the cause of the failure
+    advisory_get_installed_packages;
+    advisory_check_nonmatching_packages, fatal=>0;
+
 1;
 
 # vim: set sw=4 et:

lib/utils.pm

@@ -7,7 +7,7 @@ use Exporter;
 
 use lockapi;
 use testapi;
-our @EXPORT = qw/run_with_error_check type_safely type_very_safely desktop_vt boot_to_login_screen console_login console_switch_layout desktop_switch_layout console_loadkeys_us do_bootloader boot_decrypt check_release menu_launch_type start_cockpit repo_setup gnome_initial_setup anaconda_create_user check_desktop_clean download_modularity_tests quit_firefox/;
+our @EXPORT = qw/run_with_error_check type_safely type_very_safely desktop_vt boot_to_login_screen console_login console_switch_layout desktop_switch_layout console_loadkeys_us do_bootloader boot_decrypt check_release menu_launch_type start_cockpit repo_setup gnome_initial_setup anaconda_create_user check_desktop_clean download_modularity_tests quit_firefox advisory_get_installed_packages advisory_check_nonmatching_packages/;
 
 sub run_with_error_check {
     my ($func, $error_screen) = @_;
@@ -455,6 +455,9 @@ sub _repo_setup_updates {
     assert_script_run "createrepo .";
     # write a repo config file
     assert_script_run 'printf "[advisory]\nname=Advisory repo\nbaseurl=file:///opt/update_repo\nenabled=1\nmetadata_expire=3600\ngpgcheck=0" > /etc/yum.repos.d/advisory.repo';
+    # mark via a variable that we've set up the update repo and done
+    # all the logging stuff above
+    set_var('_ADVISORY_REPO_DONE', '1');
     # run an update now (except for upgrade tests)
     script_run "dnf -y update", 600 unless (get_var("UPGRADE"));
 }
@@ -676,3 +679,69 @@ sub quit_firefox {
     # it's a bit odd if we reach here, but could mean we quit to a
     # desktop, or the firefox_close_tabs needle went stale...
 }
+
+sub advisory_get_installed_packages {
+    # For update tests (this only works if we've been through
+    # _repo_setup_updates), figure out which packages from the update
+    # are currently installed. This is here so we can do it both in
+    # _advisory_post and post_fail_hook.
+    return unless (get_var("_ADVISORY_REPO_DONE"));
+    assert_script_run 'rpm -qa --qf "%{SOURCERPM} %{EPOCH} %{NAME}-%{VERSION}-%{RELEASE}\n" | sort -u > /tmp/allpkgs.txt';
+    # this finds lines which appear in both files
+    # http://www.unix.com/unix-for-dummies-questions-and-answers/34549-find-matching-lines-between-2-files.html
+    if (script_run 'comm -12 /tmp/allpkgs.txt /var/log/updatepkgs.txt > /var/log/testedpkgs.txt') {
+        # occasionally, for some reason, it's unhappy about sorting;
+        # we shouldn't fail the test in this case, just upload the
+        # files so we can see why...
+        upload_logs "/tmp/allpkgs.txt", failok=>1;
+        upload_logs "/var/log/updatepkgs.txt", failok=>1;
+    }
+    # we'll try and upload the output even if comm 'failed', as it
+    # does in fact still write it in some cases
+    upload_logs "/var/log/testedpkgs.txt", failok=>1;
+}
+
+sub advisory_check_nonmatching_packages {
+    # For update tests (this only works if we've been through
+    # _repo_setup_updates), figure out if we have a different version
+    # of any package from the update installed - this indicates a
+    # problem, it likely means a dep issue meant dnf installed an
+    # older version from the frozen release repo
+    my %args = (
+        fatal => 1,
+        @_
+    );
+    return unless (get_var("_ADVISORY_REPO_DONE"));
+    # if this fails in advisory_post, we don't want to do it *again*
+    # unnecessarily in post_fail_hook
+    return if (get_var("_ACNMP_DONE"));
+    script_run 'touch /tmp/installedupdatepkgs.txt';
+    script_run 'for pkg in $(cat /var/log/updatepkgnames.txt); do rpm -q $pkg && rpm -q $pkg --qf "%{SOURCERPM} %{EPOCH} %{NAME}-%{VERSION}-%{RELEASE}\n" >> /tmp/installedupdatepkgs.txt; done';
+    script_run 'sort -u -o /tmp/installedupdatepkgs.txt /tmp/installedupdatepkgs.txt';
+    # now, /tmp/installedupdatepkgs.txt is a sorted list of installed packages
+    # with the same name as packages from the update, in the same form as
+    # /var/log/updatepkgs.txt; so if any line appears in installedupdatepkgs.txt
+    # but not updatepkgs.txt, we have a problem.
+    if (script_run 'comm -23 /tmp/installedupdatepkgs.txt /var/log/updatepkgs.txt > /var/log/installednotupdatedpkgs.txt') {
+        # occasionally, for some reason, it's unhappy about sorting;
+        # we shouldn't fail the test in this case, just upload the
+        # files so we can see why...
+        upload_logs "/tmp/installedupdatepkgs.txt", failok=>1;
+        upload_logs "/var/log/updatepkgs.txt", failok=>1;
+    }
+    # this exits 1 if the file is zero-length, 0 if it's longer
+    # if it's 0, that's *BAD*: we want to upload the file and fail
+    unless (script_run 'test -s /var/log/installednotupdatedpkgs.txt') {
+        upload_logs "/var/log/installednotupdatedpkgs.txt", failok=>1;
+        upload_logs "/var/log/updatepkgs.txt", failok=>1;
+        my $message = "Package(s) from update not installed when it should have been! See installednotupdatedpkgs.txt";
+        if ($args{fatal}) {
+            set_var("_ACNMP_DONE", "1");
+            die $message;
+        }
+        else {
+            # if we're already in post_fail_hook, we don't want to die again
+            record_info $message;
+        }
+    }
+}

tests/_advisory_post.pm

@@ -5,48 +5,13 @@ use utils;
 
 sub run {
     my $self = shift;
+    $self->root_console(tty=>3);
     # figure out which packages from the update actually got installed
     # (if any) as part of this test
-    $self->root_console(tty=>3);
+    advisory_get_installed_packages;
-    assert_script_run 'rpm -qa --qf "%{SOURCERPM} %{EPOCH} %{NAME}-%{VERSION}-%{RELEASE}\n" | sort -u > /tmp/allpkgs.txt';
+    # figure out if we have a different version of any package from the
-    # this finds lines which appear in both files
+    # update installed
-    # http://www.unix.com/unix-for-dummies-questions-and-answers/34549-find-matching-lines-between-2-files.html
+    advisory_check_nonmatching_packages;
-    if (script_run 'comm -12 /tmp/allpkgs.txt /var/log/updatepkgs.txt > /var/log/testedpkgs.txt') {
-        # occasionally, for some reason, it's unhappy about sorting;
-        # we shouldn't fail the test in this case, just upload the
-        # files so we can see why...
-        upload_logs "/tmp/allpkgs.txt", failok=>1;
-        upload_logs "/var/log/updatepkgs.txt", failok=>1;
-    }
-    # we'll try and upload the output even if comm 'failed', as it
-    # does in fact still write it in some cases
-    upload_logs "/var/log/testedpkgs.txt", failok=>1;
-
-    # now, try and figure out if we have a different version of any
-    # package from the update installed - this indicates a problem,
-    # it likely means a dep issue meant dnf installed an older version
-    # from the frozen release repo
-    script_run 'touch /tmp/installedupdatepkgs.txt';
-    script_run 'for pkg in $(cat /var/log/updatepkgnames.txt); do rpm -q $pkg && rpm -q $pkg --qf "%{SOURCERPM} %{EPOCH} %{NAME}-%{VERSION}-%{RELEASE}\n" >> /tmp/installedupdatepkgs.txt; done';
-    script_run 'sort -u -o /tmp/installedupdatepkgs.txt /tmp/installedupdatepkgs.txt';
-    # now, /tmp/installedupdatepkgs.txt is a sorted list of installed packages
-    # with the same name as packages from the update, in the same form as
-    # /var/log/updatepkgs.txt; so if any line appears in installedupdatepkgs.txt
-    # but not updatepkgs.txt, we have a problem.
-    if (script_run 'comm -23 /tmp/installedupdatepkgs.txt /var/log/updatepkgs.txt > /var/log/installednotupdatedpkgs.txt') {
-        # occasionally, for some reason, it's unhappy about sorting;
-        # we shouldn't fail the test in this case, just upload the
-        # files so we can see why...
-        upload_logs "/tmp/installedupdatepkgs.txt", failok=>1;
-        upload_logs "/var/log/updatepkgs.txt", failok=>1;
-    }
-    # this exits 1 if the file is zero-length, 0 if it's longer
-    # if it's 0, that's *BAD*: we want to upload the file and fail
-    unless (script_run 'test -s /var/log/installednotupdatedpkgs.txt') {
-        upload_logs "/var/log/installednotupdatedpkgs.txt", failok=>1;
-        upload_logs "/var/log/updatepkgs.txt", failok=>1;
-        die "Package(s) from update not installed when it should have been! See installednotupdatedpkgs.txt";
-    }
 }
 
 sub test_flags {