Repository: osbuild/osbuild-composer
Pull request #606 (2020-05-12 19:30:08)

schutzbot: use -test.timeout argument
Use golang's `test.timeout` to set the timeout instead of using
ansible's `async` property. The latter doesn't respect `register` when
running into a timeout, which means there are no logs in that case.

Simplify these playbooks by setting `ignore_errors: true` on the
`command` module, so that it's easier to collect logs.

larskarlitski: Update to keep using `block` / `rescue`. I was confused in thinking that wouldn't work anymore, but it does and keeps this change much smaller. Thanks @major!

> Need a little bit of whitespace after the `60`, like `{{ test.timeout * 60 }}`

Oops. Fixed.

Patch:
diff --git a/internal/client/utils.go b/internal/client/utils.go
index e6a37be70..fee6bc311 100644
--- a/internal/client/utils.go
+++ b/internal/client/utils.go
@@ -11,7 +11,6 @@ import (
 	"net/http"
 	"sort"
 	"strconv"
-	"time"
 )
 
 type TestState struct {
@@ -32,12 +31,10 @@ func isStringInSlice(slice []string, s string) bool {
 	return false
 }
 
-func setUpTestState(socketPath string, timeout time.Duration, unitTest bool) (*TestState, error) {
+func setUpTestState(socketPath string, unitTest bool) (*TestState, error) {
 	state := TestState{unitTest: unitTest}
 
 	state.socket = &http.Client{
-		// TODO This may be too short/simple for downloading images
-		Timeout: timeout,
 		Transport: &http.Transport{
 			DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
 				return net.Dial("unix", socketPath)


Test patch:
diff --git a/internal/client/integration_test.go b/internal/client/integration_test.go
index 20bd62b44..73a1982f4 100644
--- a/internal/client/integration_test.go
+++ b/internal/client/integration_test.go
@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"os"
 	"testing"
-	"time"
 
 	"github.com/osbuild/osbuild-composer/internal/test"
 )
@@ -21,7 +20,7 @@ var testState *TestState
 // Also makes sure there is a running server to test against
 func executeTests(m *testing.M) int {
 	var err error
-	testState, err = setUpTestState("/run/weldr/api.socket", 60*time.Second, false)
+	testState, err = setUpTestState("/run/weldr/api.socket", false)
 	if err != nil {
 		fmt.Printf("ERROR: Test setup failed: %s\n", err)
 		panic(err)
diff --git a/internal/client/unit_test.go b/internal/client/unit_test.go
index 156f70356..880a8680a 100644
--- a/internal/client/unit_test.go
+++ b/internal/client/unit_test.go
@@ -12,7 +12,6 @@ import (
 	"net/http"
 	"os"
 	"testing"
-	"time"
 
 	"github.com/osbuild/osbuild-composer/internal/distro/fedoratest"
 	rpmmd_mock "github.com/osbuild/osbuild-composer/internal/mocks/rpmmd"
@@ -58,7 +57,7 @@ func executeTests(m *testing.M) int {
 		}
 	}()
 
-	testState, err = setUpTestState(socketPath, 60*time.Second, true)
+	testState, err = setUpTestState(socketPath, true)
 	if err != nil {
 		log.Fatalf("ERROR: Test setup failed: %s\n", err)
 	}
diff --git a/schutzbot/test_runner_base.yml b/schutzbot/test_runner_base.yml
index 0b9ad5736..73ca336c3 100644
--- a/schutzbot/test_runner_base.yml
+++ b/schutzbot/test_runner_base.yml
@@ -3,12 +3,10 @@
 - block:
 
     - name: "Run {{ test.name }} test"
-      command: "{{ tests_path }}/{{ test.name }} -test.v"
+      command: "{{ tests_path }}/{{ test.name }} -test.v -test.timeout {{ test.timeout * 60 }}s"
       args:
         chdir: "{{ tests_working_directory }}"
-      register: async_test
-      async: "{{ test.timeout * 60 }}"
-      poll: "{{ polling_interval }}"
+      register: test_cmd
 
     - name: "Mark {{ test.name }} as passed"
       set_fact:
@@ -29,8 +27,8 @@
           Logs from {{ test.name }}
           ----------------------------------------------------------------------
           stderr:
-          {{ async_test.stderr }}
+          {{ test_cmd.stderr }}
           ----------------------------------------------------------------------
           stdout:
-          {{ async_test.stdout }}
+          {{ test_cmd.stdout }}
 
diff --git a/schutzbot/test_runner_image.yml b/schutzbot/test_runner_image.yml
index 43d2b2d69..a22582307 100644
--- a/schutzbot/test_runner_image.yml
+++ b/schutzbot/test_runner_image.yml
@@ -24,7 +24,7 @@
     # upset and nobody likes it when that happens.
     - name: "Run image tests"
       command: |
-        {{ image_test_executable }} -test.v \
+        {{ image_test_executable }} -test.v -test.timeout {{image_test_timeout * 60}}s \
           {% for test_case in osbuild_composer_image_test_cases %}
             {% if loop.last %}
           {{ image_test_case_path }}/{{ test_case_prefix }}-{{ test_case }}
@@ -34,9 +34,7 @@
           {% endfor %}
       args:
         chdir: "{{ tests_working_directory }}"
-      register: async_test
-      async: "{{ image_test_timeout * 60 }}"
-      poll: "{{ polling_interval }}"
+      register: test_cmd
 
     - name: "Mark image tests as passed"
       set_fact:
@@ -57,8 +55,8 @@
           Logs from image test
           ----------------------------------------------------------------------
           stderr:
-          {{ async_test.stderr }}
+          {{ test_cmd.stderr }}
           ----------------------------------------------------------------------
           stdout:
-          {{ async_test.stdout }}
+          {{ test_cmd.stdout }}