tests/image: switch the implementation to go testing framework

We're currently rewriting all the integration tests to use the Go
testing framework. This commit does the switch for the image tests.

I decided not to use the testing framework in functions which are
not directly tight to testing (booting images, running osbuild). I think
it's reasonable to use classic error handling there and propagate the errors
to places directly tight to testing and use the testing library.
This enables us to reuse the code in different part of projects if needed.

.travis.yml

@@ -25,5 +25,5 @@ script:
   - eval "$(gimme 1.13.x)"
   # ubuntu's rpm package sets dbpath to ~/.rpmdb, which makes rpm fail...
   - sudo sh -c 'mkdir /etc/rpm; echo "%_dbpath /var/lib/rpm" > /etc/rpm/macros'
-  - go build -tags travis ./cmd/osbuild-image-tests
+  - go test -c -tags travis,integration -o osbuild-image-tests ./cmd/osbuild-image-tests
-  - sudo ./osbuild-image-tests --distro $TRAVIS_JOB_NAME
+  - sudo ./osbuild-image-tests -test.v --distro $TRAVIS_JOB_NAME

Makefile

@@ -11,6 +11,7 @@ build:
 	go test -c -tags=integration -o osbuild-weldr-tests ./internal/weldrcheck/
 	go test -c -tags=integration -o osbuild-dnf-json-tests ./cmd/osbuild-dnf-json-tests/main_test.go
 	go test -c -tags=integration -o osbuild-rcm-tests ./cmd/osbuild-rcm-tests/main_test.go
+	go test -c -tags=integration -o osbuild-image-tests ./cmd/osbuild-image-tests/main_test.go
 
 .PHONY: install
 install:

cmd/osbuild-image-tests/main_test.go

@@ -1,3 +1,5 @@
+// +build integration
+
 package main
 
 import (
@@ -13,14 +15,18 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"testing"
 	"time"
 
-	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/osbuild/osbuild-composer/internal/common"
 	"github.com/osbuild/osbuild-composer/internal/distro"
 )
 
+var distroArg = flag.String("distro", "", "which distro tests to run")
+
 type testcaseStruct struct {
 	ComposeRequest struct {
 		Distro   string
@@ -43,19 +49,15 @@ func runOsbuild(manifest []byte, store string) (string, error) {
 	var outBuffer bytes.Buffer
 	cmd.Stdout = &outBuffer
 
-	log.Print("[osbuild] running")
 	err := cmd.Run()
 
 	if err != nil {
-		log.Print("[osbuild] failed")
 		if _, ok := err.(*exec.ExitError); ok {
 			return "", fmt.Errorf("running osbuild failed: %s", outBuffer.String())
 		}
 		return "", fmt.Errorf("running osbuild failed from an unexpected reason: %v", err)
 	}
 
-	log.Print("[osbuild] succeeded")
-
 	var result struct {
 		OutputID string `json:"output_id"`
 	}
@@ -92,12 +94,10 @@ func splitExtension(path string) (string, string) {
 
 // testImageInfo runs image-info on image specified by imageImage and
 // compares the result with expected image info
-func testImageInfo(imagePath string, rawImageInfoExpected []byte) error {
+func testImageInfo(t *testing.T, imagePath string, rawImageInfoExpected []byte) {
 	var imageInfoExpected interface{}
 	err := json.Unmarshal(rawImageInfoExpected, &imageInfoExpected)
-	if err != nil {
+	require.Nilf(t, err, "cannot decode expected image info: %v", err)
-		return fmt.Errorf("cannot decode expected image info: %v", err)
-	}
 
 	cmd := exec.Command(imageInfoPath, imagePath)
 	cmd.Stderr = os.Stderr
@@ -105,26 +105,16 @@ func testImageInfo(imagePath string, rawImageInfoExpected []byte) error {
 	cmd.Stdout = writer
 
 	err = cmd.Start()
-	if err != nil {
+	require.Nilf(t, err, "image-info cannot start: %v", err)
-		return fmt.Errorf("image-info cannot start: %v", err)
-	}
 
 	var imageInfoGot interface{}
 	err = json.NewDecoder(reader).Decode(&imageInfoGot)
-	if err != nil {
+	require.Nilf(t, err, "decoding image-info output failed: %v", err)
-		return fmt.Errorf("decoding image-info output failed: %v", err)
-	}
 
 	err = cmd.Wait()
-	if err != nil {
+	require.Nilf(t, err, "running image-info failed: %v", err)
-		return fmt.Errorf("running image-info failed: %v", err)
-	}
 
-	if diff := cmp.Diff(imageInfoExpected, imageInfoGot); diff != "" {
+	assert.Equal(t, imageInfoExpected, imageInfoGot)
-		return fmt.Errorf("image info differs:\n%s", diff)
-	}
-
-	return nil
 }
 
 type timeoutError struct{}
@@ -182,21 +172,24 @@ func trySSHOnce(ns netNS) error {
 // testSSH tests the running image using ssh.
 // It tries 20 attempts before giving up. If a major error occurs, it might
 // return earlier.
-func testSSH(ns netNS) error {
+func testSSH(t *testing.T, ns netNS) {
 	const attempts = 20
 	for i := 0; i < attempts; i++ {
 		err := trySSHOnce(ns)
 		if err == nil {
-			return nil
+			// pass the test
+			return
 		}
+
+		// if any other error than the timeout one happened, fail the test immediately
 		if _, ok := err.(*timeoutError); !ok {
-			return err
+			t.Fatal(err)
 		}
 
 		time.Sleep(10 * time.Second)
 	}
 
-	return fmt.Errorf("ssh test failure, %d attempts were made", attempts)
+	t.Errorf("ssh test failure, %d attempts were made", attempts)
 }
 
 // testBoot tests if the image is able to successfully boot
@@ -204,68 +197,58 @@ func testSSH(ns netNS) error {
 // The test passes if the function is able to connect to the image via ssh
 // in defined number of attempts and systemd-is-running returns running
 // or degraded status.
-func testBoot(imagePath string, bootType string, outputID string) error {
+func testBoot(t *testing.T, imagePath string, bootType string, outputID string) {
-	return withNetworkNamespace(func(ns netNS) error {
+	err := withNetworkNamespace(func(ns netNS) error {
 		switch bootType {
 		case "qemu":
 			fallthrough
 		case "qemu-extract":
 			return withBootedQemuImage(imagePath, ns, func() error {
-				return testSSH(ns)
+				testSSH(t, ns)
+				return nil
 			})
 		case "nspawn":
 			return withBootedNspawnImage(imagePath, outputID, ns, func() error {
-				return testSSH(ns)
+				testSSH(t, ns)
+				return nil
 			})
 		case "nspawn-extract":
 			return withExtractedTarArchive(imagePath, func(dir string) error {
 				return withBootedNspawnDirectory(dir, outputID, ns, func() error {
-					return testSSH(ns)
+					testSSH(t, ns)
+					return nil
 				})
 			})
 		default:
 			panic("unknown boot type!")
 		}
 	})
+
+	require.Nil(t, err)
 }
 
 // testImage performs a series of tests specified in the testcase
 // on an image
-func testImage(testcase testcaseStruct, imagePath, outputID string) error {
+func testImage(t *testing.T, testcase testcaseStruct, imagePath, outputID string) {
 	if testcase.ImageInfo != nil {
-		log.Print("[image info sub-test] running")
+		t.Run("image info", func(t *testing.T) {
-		err := testImageInfo(imagePath, testcase.ImageInfo)
+			testImageInfo(t, imagePath, testcase.ImageInfo)
-		if err != nil {
+		})
-			log.Print("[image info sub-test] failed")
-			return err
-		}
-		log.Print("[image info sub-test] succeeded")
-	} else {
-		log.Print("[image info sub-test] not defined, skipping")
 	}
 
 	if testcase.Boot != nil {
-		log.Print("[boot sub-test] running")
+		t.Run("boot", func(t *testing.T) {
-		err := testBoot(imagePath, testcase.Boot.Type, outputID)
+			testBoot(t, imagePath, testcase.Boot.Type, outputID)
-		if err != nil {
+		})
-			log.Print("[boot sub-test] failed")
-			return err
-		}
-		log.Print("[boot sub-test] succeeded")
-	} else {
-		log.Print("[boot sub-test] not defined, skipping")
 	}
-
-	return nil
 }
 
 // runTestcase builds the pipeline specified in the testcase and then it
 // tests the result
-func runTestcase(testcase testcaseStruct) error {
+func runTestcase(t *testing.T, testcase testcaseStruct) {
 	store, err := ioutil.TempDir("/var/tmp", "osbuild-image-tests-")
-	if err != nil {
+	require.Nilf(t, err, "cannot create temporary store: %v", err)
-		return fmt.Errorf("cannot create temporary store: %v", err)
+
-	}
 	defer func() {
 		err := os.RemoveAll(store)
 		if err != nil {
@@ -274,9 +257,7 @@ func runTestcase(testcase testcaseStruct) error {
 	}()
 
 	outputID, err := runOsbuild(testcase.Manifest, store)
-	if err != nil {
+	require.Nil(t, err)
-		return err
-	}
 
 	imagePath := fmt.Sprintf("%s/refs/%s/%s", store, outputID, testcase.ComposeRequest.Filename)
 
@@ -285,14 +266,13 @@ func runTestcase(testcase testcaseStruct) error {
 	if ex == ".xz" {
 		_, ex = splitExtension(base)
 		if ex != ".tar" {
-			if err := extractXZ(imagePath); err != nil {
+			err := extractXZ(imagePath)
-				return err
+			require.Nil(t, err)
-			}
 			imagePath = base
 		}
 	}
 
-	return testImage(testcase, imagePath, outputID)
+	testImage(t, testcase, imagePath, outputID)
 }
 
 // getAllCases returns paths to all testcases in the testcase directory
@@ -316,78 +296,53 @@ func getAllCases() ([]string, error) {
 }
 
 // runTests opens, parses and runs all the specified testcases
-func runTests(cases []string, d string) error {
+func runTests(t *testing.T, cases []string, d string) {
 	for _, path := range cases {
-		f, err := os.Open(path)
+		t.Run(path, func(t *testing.T) {
-		if err != nil {
+			f, err := os.Open(path)
-			return fmt.Errorf("%s: cannot open test case: %v", path, err)
+			require.Nilf(t, err, "%s: cannot open test case: %v", path, err)
-		}
 
-		var testcase testcaseStruct
+			var testcase testcaseStruct
-		err = json.NewDecoder(f).Decode(&testcase)
+			err = json.NewDecoder(f).Decode(&testcase)
-		if err != nil {
+			require.Nilf(t, err, "%s: cannot decode test case: %v", path, err)
-			return fmt.Errorf("%s: cannot decode test case: %v", path, err)
-		}
 
-		currentArch := common.CurrentArch()
+			currentArch := common.CurrentArch()
-		if testcase.ComposeRequest.Arch != currentArch {
+			if testcase.ComposeRequest.Arch != currentArch {
-			log.Printf("%s: skipping, the required arch is %s, the current arch is %s", path, testcase.ComposeRequest.Arch, currentArch)
+				t.Skipf("the required arch is %s, the current arch is %s", testcase.ComposeRequest.Arch, currentArch)
-			continue
-		}
-
-		if d != "" {
-			if testcase.ComposeRequest.Distro != d {
-				log.Printf("%s: skipping, the required distro is %s, the passed distro is %s", path, testcase.ComposeRequest.Distro, d)
-				continue
-			}
-		} else {
-			hostDistroName, err := distro.GetHostDistroName()
-			if err != nil {
-				return fmt.Errorf("cannot get host distro name: %v", err)
 			}
 
-			// TODO: forge distro name for now
+			if d != "" {
-			if strings.HasPrefix(hostDistroName, "fedora") {
+				if testcase.ComposeRequest.Distro != d {
-				hostDistroName = "fedora-30"
+					t.Skipf("the required distro is %s, the passed distro is %s", testcase.ComposeRequest.Distro, d)
+				}
+			} else {
+				hostDistroName, err := distro.GetHostDistroName()
+				require.Nil(t, err)
+
+				// TODO: forge distro name for now
+				if strings.HasPrefix(hostDistroName, "fedora") {
+					hostDistroName = "fedora-30"
+				}
+
+				if testcase.ComposeRequest.Distro != hostDistroName {
+					t.Skipf("the required distro is %s, the host distro is %s", testcase.ComposeRequest.Distro, hostDistroName)
+				}
 			}
 
-			if testcase.ComposeRequest.Distro != hostDistroName {
+			runTestcase(t, testcase)
-				log.Printf("%s: skipping, the required distro is %s, the host distro is %s", path, testcase.ComposeRequest.Distro, hostDistroName)
+		})
-				continue
-			}
-		}
-
-		log.Printf("%s: RUNNING", path)
-
-		err = runTestcase(testcase)
-		if err != nil {
-			log.Printf("%s: FAILURE\nReason: %v", path, err)
-		} else {
-			log.Printf("%s: SUCCESS", path)
-		}
 
 	}
-
-	return nil
 }
 
-func main() {
+func TestImages(t *testing.T) {
-	d := flag.String("distro", "", "which distro tests to run")
-	flag.Parse()
 	cases := flag.Args()
-
 	// if no cases were specified, run the default set
 	if len(cases) == 0 {
 		var err error
 		cases, err = getAllCases()
-		if err != nil {
+		require.Nil(t, err)
-			log.Fatalf("searching for testcases failed: %v", err)
-		}
 	}
 
-	err := runTests(cases, *d)
+	runTests(t, cases, *distroArg)
-
-	if err != nil {
-		log.Fatalf("error occured while running tests: %v", err)
-	}
 }

golang-github-osbuild-composer.spec

@@ -66,7 +66,6 @@ export GOFLAGS=-mod=vendor
 %endif
 %gobuild -o _bin/osbuild-composer %{goipath}/cmd/osbuild-composer
 %gobuild -o _bin/osbuild-worker %{goipath}/cmd/osbuild-worker
-%gobuild -o _bin/osbuild-image-tests %{goipath}/cmd/osbuild-image-tests
 
 # Build test binaries with `go test -c`, so that they can take advantage of
 # golang's testing package. The golang rpm macros don't support building them
@@ -78,6 +77,7 @@ go test -c -tags=integration -ldflags="${TEST_LDFLAGS}" -o _bin/osbuild-tests %{
 go test -c -tags=integration -ldflags="${TEST_LDFLAGS}" -o _bin/osbuild-dnf-json-tests %{goipath}/cmd/osbuild-dnf-json-tests
 go test -c -tags=integration -ldflags="${TEST_LDFLAGS}" -o _bin/osbuild-weldr-tests %{goipath}/internal/weldrcheck/
 go test -c -tags=integration -ldflags="${TEST_LDFLAGS}" -o _bin/osbuild-rcm-tests %{goipath}/cmd/osbuild-rcm-tests
+go test -c -tags=integration -ldflags="${TEST_LDFLAGS}" -o _bin/osbuild-image-tests %{goipath}/cmd/osbuild-image-tests
 
 %install
 install -m 0755 -vd                                         %{buildroot}%{_libexecdir}/osbuild-composer