ext/featurefmt: Refactor featurefmt testing code

1. Featurefmt testing code is moved to featurefmttest package.
2. Featurefmt now can be tested against a csv file, which contains the
expected package information result.

ext/featurefmt/apk/apk_test.go

@@ -18,7 +18,7 @@ import (
 	"testing"
 
 	"github.com/coreos/clair/database"
-	"github.com/coreos/clair/ext/featurefmt"
+	"github.com/coreos/clair/ext/featurefmt/featurefmttest"
 	"github.com/coreos/clair/ext/versionfmt/dpkg"
 	"github.com/coreos/clair/pkg/tarutil"
 )
@@ -42,13 +42,13 @@ func TestAPKFeatureDetection(t *testing.T) {
 		testFeatures[i].VersionFormat = dpkg.ParserName
 	}
 
-	testData := []featurefmt.TestData{
+	testData := []featurefmttest.TestData{
 		{
 			Features: testFeatures,
 			Files: tarutil.FilesMap{
-				"lib/apk/db/installed": featurefmt.LoadFileForTest("apk/testdata/installed"),
+				"lib/apk/db/installed": featurefmttest.LoadFileForTest("apk/testdata/installed"),
 			},
 		},
 	}
-	featurefmt.TestLister(t, &lister{}, testData)
+	featurefmttest.TestLister(t, &lister{}, testData)
 }

ext/featurefmt/driver.go

@@ -17,14 +17,9 @@
 package featurefmt
 
 import (
-	"io/ioutil"
-	"path/filepath"
-	"runtime"
 	"sync"
-	"testing"
 
 	log "github.com/sirupsen/logrus"
-	"github.com/stretchr/testify/assert"
 
 	"github.com/coreos/clair/database"
 	"github.com/coreos/clair/pkg/tarutil"
@@ -134,30 +129,3 @@ func ListListers() []database.Detector {
 	}
 	return r
 }
-
-// TestData represents the data used to test an implementation of Lister.
-type TestData struct {
-	Files    tarutil.FilesMap
-	Features []database.Feature
-}
-
-// LoadFileForTest can be used in order to obtain the []byte contents of a file
-// that is meant to be used for test data.
-func LoadFileForTest(name string) []byte {
-	_, filename, _, _ := runtime.Caller(0)
-	d, _ := ioutil.ReadFile(filepath.Join(filepath.Dir(filename)) + "/" + name)
-	return d
-}
-
-// TestLister runs a Lister on each provided instance of TestData and asserts
-// the ouput to be equal to the expected output.
-func TestLister(t *testing.T, l Lister, testData []TestData) {
-	for _, td := range testData {
-		featureVersions, err := l.ListFeatures(td.Files)
-		if assert.Nil(t, err) && assert.Len(t, featureVersions, len(td.Features)) {
-			for _, expectedFeature := range td.Features {
-				assert.Contains(t, featureVersions, expectedFeature)
-			}
-		}
-	}
-}

ext/featurefmt/package_info.go

@@ -0,0 +1,56 @@
+package featurefmt
+
+import (
+	"github.com/coreos/clair/database"
+	"github.com/deckarep/golang-set"
+)
+
+// PackageInfo is the extracted raw information from the package managers that
+// can be converted to a feature.
+type PackageInfo struct {
+	PackageName    string
+	PackageVersion string
+	SourceName     string
+	SourceVersion  string
+}
+
+// Reset defaults the internal string fields to empty strings.
+func (pkg *PackageInfo) Reset() {
+	pkg.PackageName = ""
+	pkg.PackageVersion = ""
+	pkg.SourceName = ""
+	pkg.SourceVersion = ""
+}
+
+func (pkg *PackageInfo) asFeature(versionFormat string) database.Feature {
+	feature := database.Feature{
+		Name:          pkg.PackageName,
+		Version:       pkg.PackageVersion,
+		VersionFormat: versionFormat,
+	}
+
+	if pkg.SourceName != "" {
+		parent := database.Feature{
+			Name:          pkg.SourceName,
+			Version:       pkg.SourceVersion,
+			VersionFormat: versionFormat,
+		}
+
+		if parent != feature {
+			feature.Parent = &parent
+		}
+	}
+
+	return feature
+}
+
+// PackageSetToFeatures converts a package set to feature slice
+func PackageSetToFeatures(versionFormat string, pkgs mapset.Set) []database.Feature {
+	features := make([]database.Feature, 0, pkgs.Cardinality())
+	for pkg := range pkgs.Iter() {
+		p := pkg.(PackageInfo)
+		features = append(features, p.asFeature(versionFormat))
+	}
+
+	return features
+}

ext/featurefmt/rpm/rpm_test.go

@@ -18,13 +18,13 @@ import (
 	"testing"
 
 	"github.com/coreos/clair/database"
-	"github.com/coreos/clair/ext/featurefmt"
+	"github.com/coreos/clair/ext/featurefmt/featurefmttest"
 	"github.com/coreos/clair/ext/versionfmt/rpm"
 	"github.com/coreos/clair/pkg/tarutil"
 )
 
 func TestRpmFeatureDetection(t *testing.T) {
-	testData := []featurefmt.TestData{
+	testData := []featurefmttest.TestData{
 		// Test a CentOS 7 RPM database
 		// Memo: Use the following command on a RPM-based system to shrink a database: rpm -qa --qf "%{NAME}\n" |tail -n +3| xargs rpm -e --justdb
 		{
@@ -43,10 +43,10 @@ func TestRpmFeatureDetection(t *testing.T) {
 				},
 			},
 			Files: tarutil.FilesMap{
-				"var/lib/rpm/Packages": featurefmt.LoadFileForTest("rpm/testdata/Packages"),
+				"var/lib/rpm/Packages": featurefmttest.LoadFileForTest("rpm/testdata/Packages"),
 			},
 		},
 	}
 
-	featurefmt.TestLister(t, &lister{}, testData)
+	featurefmttest.TestLister(t, &lister{}, testData)
 }

ext/featurefmt/testutil.go

@@ -0,0 +1,118 @@
+// Copyright 2018 clair authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package featurefmt contains utility functions for featurefmt tests
+package featurefmt
+
+import (
+	"io/ioutil"
+	"path/filepath"
+	"runtime"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/coreos/clair/database"
+	"github.com/coreos/clair/pkg/tarutil"
+)
+
+// LoadFileForTest can be used in order to obtain the []byte contents of a file
+// that is meant to be used for test data.
+func LoadFileForTest(name string) []byte {
+	_, filename, _, _ := runtime.Caller(0)
+	path := filepath.Join(filepath.Dir(filename), name)
+	d, err := ioutil.ReadFile(path)
+	if err != nil {
+		panic(err)
+	}
+
+	return d
+}
+
+func loadTestFiles(testFilePaths map[string]string) tarutil.FilesMap {
+	m := tarutil.FilesMap{}
+	for tarPath, fsPath := range testFilePaths {
+		m[tarPath] = LoadFileForTest(fsPath)
+	}
+
+	return m
+}
+
+// TestCase is used by the RunTest function to execute.
+type TestCase struct {
+	Name           string
+	FilePaths      map[string]string
+	ExpectedResult []PackageInfo
+}
+
+// RunTest runs a featurefmt test by loading the package info database files and
+// the expected packages.
+func RunTest(t *testing.T, test TestCase, lister Lister, expectedVersionFormat string) {
+	t.Run(test.Name, func(t *testing.T) {
+		filesMap := loadTestFiles(test.FilePaths)
+		expected := test.ExpectedResult
+		features, err := lister.ListFeatures(filesMap)
+		require.Nil(t, err)
+		visited := map[PackageInfo]bool{}
+		// we only enforce the unique packages to match, the result features
+		// should be always deduplicated.
+		for _, pkg := range expected {
+			visited[pkg] = false
+		}
+
+		assert.Len(t, features, len(visited))
+		for _, f := range features {
+			assert.Equal(t, expectedVersionFormat, f.VersionFormat)
+			if f.Parent != nil {
+				// currently we don't have more than 2 levels deep features.
+				assert.Equal(t, expectedVersionFormat, f.Parent.VersionFormat)
+			}
+
+			pkg := convertToPackageInfo(&f)
+			if ok, found := visited[pkg]; ok {
+				assert.Fail(t, "duplicated features is not allowed", "feature=%#v", f, pkg)
+			} else if !found {
+				assert.Fail(t, "unexpected feature", "feature = %#v", pkg)
+			}
+
+			visited[pkg] = true
+		}
+
+		missingPackages := []PackageInfo{}
+		for pkg, ok := range visited {
+			if !ok {
+				missingPackages = append(missingPackages, pkg)
+			}
+		}
+
+		assert.Len(t, missingPackages, 0, "missing packages")
+	})
+}
+
+func convertToPackageInfo(feature *database.Feature) PackageInfo {
+	pkg := PackageInfo{
+		PackageName:    feature.Name,
+		PackageVersion: feature.Version,
+	}
+
+	// Since in the actual package manager metadata file, there's no explicit
+	// tree structure, the features are converted to compare the metadata only.
+	if feature.Parent != nil {
+		pkg.SourceName = feature.Parent.Name
+		pkg.SourceVersion = feature.Parent.Version
+	}
+
+	return pkg
+}