From a8fb32225721038451015fc255b92d1b894858cc Mon Sep 17 00:00:00 2001
From: Josh Cooper <joshcooper@users.noreply.github.com>
Date: Mon, 22 Jan 2024 17:24:59 -0800
Subject: [PATCH 1/2] Always run post-suite if provision succeeded

The `ci:test:aio` rake task would skip the `post-suite` if any presuites or
tests failed. Now the `post-suite` is protected in a nested ensure block so if
`provision` succeeds, then the `post-suite` is guaranteed to be called. This is
important in order to capture failures when installing the agent during the
`pre-suite`.

And due to the nested ensure, if the `post-suite` fails, then `destroy` will
also be guaranteed to be called.

```
$ export BEAKER_PUPPET_VERSION=/home/josh/work/beaker-puppet
$ bundle update
$ bundle exec rake 'ci:test:aio[true]' SHA=8.4.0 TESTS=tests/load_libfacter.rb HOSTS=hosts.yaml
...
Begin tests/load_libfacter.rb

C100161: Ruby can load libfacter without raising an error
RuntimeError: Whoops
/home/josh/work/facter/acceptance/tests/load_libfacter.rb:5
...
beaker exec post-suite
...
Begin teardown/common/099_Archive_Logs.rb
```
---
 tasks/ci.rake | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tasks/ci.rake b/tasks/ci.rake
index 2ec9378..cb55c74 100644
--- a/tasks/ci.rake
+++ b/tasks/ci.rake
@@ -315,10 +315,14 @@ def beaker_suite(type)
   beaker(:provision)
 
   begin
-    beaker(:exec, 'pre-suite', '--preserve-state', '--pre-suite', pre_suites(type))
-    beaker(:exec, 'pre-suite', '--preserve-state')
-    beaker(:exec, ENV.fetch('TESTS', nil))
-    beaker(:exec, 'post-suite')
+    begin
+      beaker(:exec, 'pre-suite', '--preserve-state', '--pre-suite', pre_suites(type))
+      beaker(:exec, 'pre-suite', '--preserve-state')
+
+      beaker(:exec, ENV.fetch('TESTS', nil))
+    ensure
+      beaker(:exec, 'post-suite')
+    end
   ensure
     preserve_hosts = ENV['OPTIONS'].include?('--preserve-hosts=always') if ENV['OPTIONS']
     beaker(:destroy) unless preserve_hosts

From 1d6b1d9d4066493e261a84e0879e839f3d92963a Mon Sep 17 00:00:00 2001
From: Josh Cooper <joshcooper@users.noreply.github.com>
Date: Mon, 22 Jan 2024 17:32:55 -0800
Subject: [PATCH 2/2] Ensure hosts are destroyed if post-suite fails

When running `rake ci:test:aio[true]`, if the post-suite failed and we're not
preserving hosts, then the `beaker destroy` command would be skipped. Use a
nested ensure so the failure of post-suite doesn't prevent `beaker destroy` from
being called.

The nested ensure also guarantees that the `post-suite` will be called even if
the `pre-suite` fails, which can happen if the agent fails to install. This
matches the behavior for the "non-retry" version of the `ci:test:aio` rake task.

```
$ git diff
diff --git a/acceptance/teardown/common/099_Archive_Logs.rb b/acceptance/teardown/common/099_Archive_Logs.rb
index d940e2bea..2302633e1 100644
--- a/acceptance/teardown/common/099_Archive_Logs.rb
+++ b/acceptance/teardown/common/099_Archive_Logs.rb
@@ -1,5 +1,7 @@
 require 'date'

+raise "whoops"
+
 def file_glob(host, path)
   result = on(host, "ls #{path}", :acceptable_exit_codes => [0, 2])
   return [] if result.exit_code != 0

$ export BEAKER_PUPPET_VERSION=/home/josh/work/beaker-puppet
$ bundle update
...
$ bundle exec rake 'ci:test:aio[true]' SHA=8.4.0 TESTS=tests/load_libfacter.rb HOSTS=hosts.yaml
...
Begin teardown/common/099_Archive_Logs.rb
RuntimeError: whoops
...
beaker destroy
```
---
 tasks/ci.rake | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/tasks/ci.rake b/tasks/ci.rake
index cb55c74..a4d03dc 100644
--- a/tasks/ci.rake
+++ b/tasks/ci.rake
@@ -334,23 +334,26 @@ def beaker_suite_retry(type)
   beaker(:provision)
 
   begin
-    beaker(:exec, 'pre-suite', '--preserve-state', '--pre-suite', pre_suites(type))
-    beaker(:exec, 'pre-suite', '--preserve-state')
-
     begin
-      json_results_file = Tempfile.new
-      beaker(:exec, ENV.fetch('TESTS', nil), '--test-results-file', json_results_file.path)
-    rescue RuntimeError => e
-      puts "ERROR: #{e.message}"
-      tests_to_rerun = JSON.load(File.read(json_results_file.path))
-      raise e if tests_to_rerun.nil? || tests_to_rerun.empty?
-
-      puts '*** Retrying the following:'
-      puts tests_to_rerun.map { |spec| "  #{spec}" }
-      beaker(:exec, tests_to_rerun.map { |str| "#{str}" }.join(','))
+      beaker(:exec, 'pre-suite', '--preserve-state', '--pre-suite', pre_suites(type))
+      beaker(:exec, 'pre-suite', '--preserve-state')
+
+      begin
+        json_results_file = Tempfile.new
+        beaker(:exec, ENV.fetch('TESTS', nil), '--test-results-file', json_results_file.path)
+      rescue RuntimeError => e
+        puts "ERROR: #{e.message}"
+        tests_to_rerun = JSON.load(File.read(json_results_file.path))
+        raise e if tests_to_rerun.nil? || tests_to_rerun.empty?
+
+        puts '*** Retrying the following:'
+        puts tests_to_rerun.map { |spec| "  #{spec}" }
+        beaker(:exec, tests_to_rerun.map { |str| "#{str}" }.join(','))
+      end
+    ensure
+      beaker(:exec, 'post-suite')
     end
   ensure
-    beaker(:exec, 'post-suite')
     preserve_hosts = ENV['OPTIONS'].include?('--preserve-hosts=always') if ENV['OPTIONS']
     beaker(:destroy) unless preserve_hosts
   end