[testsharder] Introduce test spec validation

Test: ran testsharder locally; tests sharded as intended.
Bug: IN-497
Bug: IN-673
Change-Id: I3c3c13ae3f2951f9b2d7250c17c241c13ef4db7f
diff --git a/cmd/testsharder/algorithm_test.go b/cmd/testsharder/algorithm_test.go
index 57b6a6e..4b96a14 100644
--- a/cmd/testsharder/algorithm_test.go
+++ b/cmd/testsharder/algorithm_test.go
@@ -46,7 +46,7 @@
 		shards := makeShards([]testexec.TestSpec{spec1}, "prefix")
 		expect := []*testexec.Shard{
 			{
-				Name: "prefix-0000",
+				Name:  "prefix-0000",
 				Tests: []testexec.Test{test1},
 				Env: testexec.Environment{
 					Device: device1,
@@ -74,7 +74,7 @@
 				},
 			},
 			{
-				Name: "0001",
+				Name:  "0001",
 				Tests: []testexec.Test{test2},
 				Env: testexec.Environment{
 					Device: device2,
diff --git a/cmd/testsharder/main.go b/cmd/testsharder/main.go
index 8dcbf27..9d0783d 100644
--- a/cmd/testsharder/main.go
+++ b/cmd/testsharder/main.go
@@ -6,24 +6,18 @@
 import (
 	"encoding/json"
 	"flag"
-	"io/ioutil"
 	"log"
 	"os"
+	"path/filepath"
 
+	"fuchsia.googlesource.com/infra/infra/fuchsia"
 	"fuchsia.googlesource.com/infra/infra/fuchsia/testexec"
 )
 
-// Flags.
 var (
 	// The path to the Fuchsia build output directory.
 	fuchsiaBuildDir string
 
-	// The filepath containing a set of supported platforms.
-	platforms string
-
-	// The architecture the tests were built for.
-	targetArch string
-
 	// Human-friendly prefix for shard names.
 	shardPrefix string
 
@@ -32,11 +26,7 @@
 )
 
 func init() {
-	// TODO(mknyszek): Extend this in the future with options that influence
-	// the sharding algorithm/policy.
 	flag.StringVar(&fuchsiaBuildDir, "fuchsia-build-dir", "", "(required) path to the fuchsia build output directory")
-	flag.StringVar(&platforms, "platforms-file", "", "(required) path to a file containing a JSON of valid platforms")
-	flag.StringVar(&targetArch, "target-arch", "", "(required) target architecture")
 	flag.StringVar(&shardPrefix, "shard-prefix", "", "human-friendly prefix for shard names")
 	flag.StringVar(&outputFile, "output-file", "", "path to a file which will contain the shards as JSON, default is stdout")
 }
@@ -44,42 +34,34 @@
 func main() {
 	flag.Parse()
 
-	// Validate options.
 	if fuchsiaBuildDir == "" {
 		log.Fatal("must specify a Fuchsia build output directory")
 	}
-	if platforms == "" {
-		log.Fatal("must specify a file with a set of valid platforms")
-	}
-	if targetArch == "" {
-		log.Fatal("must specify a target CPU architecture")
-	}
-
-	// Load platforms file.
-	bytes, err := ioutil.ReadFile(platforms)
-	if err != nil {
-		log.Fatal("failed to read platforms file:", err)
-	}
-	platforms := testexec.TestDevicePlatforms{}
-	if err := json.Unmarshal(bytes, &platforms); err != nil {
-		log.Fatal("failed to decode platforms file:", err)
-	}
 
 	// Load spec files.
-	specs, err := testexec.LoadTestSpecs(fuchsiaBuildDir)
+	pkgManifestPath := filepath.Join(fuchsiaBuildDir, fuchsia.PackageManifestName)
+	pkgManifest, err := fuchsia.LoadPackageManifest(pkgManifestPath)
 	if err != nil {
-		log.Fatalf("failed to load test specs in %s: %s", fuchsiaBuildDir, err.Error())
+		log.Fatalf("failed to load package manifest from %s: %v", pkgManifestPath, err)
 	}
-	if len(specs) == 0 {
-		log.Fatal("no specs found in directory, exiting...")
-	}
-	for _, spec := range specs {
-		if err := spec.Validate(targetArch, platforms); err != nil {
-			log.Fatal("found invalid spec: ", err)
-		}
+	specs, err := testexec.LoadTestSpecs(fuchsiaBuildDir, *pkgManifest)
+	if err != nil {
+		log.Fatalf("failed to load test specs from %s: %v", fuchsiaBuildDir, err)
 	}
 
-	// Create shards.
+	// Load test platform environments.
+	platformManifestPath := filepath.Join(fuchsiaBuildDir, fuchsia.PlatformManifestName)
+	platforms, err := testexec.LoadPlatforms(platformManifestPath)
+	if err != nil {
+		log.Fatalf("failed to load test platforms from %s: %v", platformManifestPath, err)
+	}
+
+	// Verify that the produced specs specify valid test environments.
+	if err = testexec.ValidateTestSpecs(specs, platforms); err != nil {
+		log.Fatalf("invalid test environment(s) found: %v", err)
+	}
+
+	// Create shards and write them to an output file if specifed, else stdout.
 	shards := &testexec.Shards{
 		Shards: makeShards(specs, shardPrefix),
 	}
@@ -89,12 +71,11 @@
 		var err error
 		f, err = os.Create(outputFile)
 		if err != nil {
-			log.Fatalf("unable to create %s: %s", outputFile, err.Error())
+			log.Fatalf("unable to create %s: %v", outputFile, err)
 		}
 		defer f.Close()
 	}
 
-	// Write shards to output.
 	encoder := json.NewEncoder(f)
 	encoder.SetIndent("", "  ")
 	if err := encoder.Encode(&shards); err != nil {
diff --git a/fuchsia/constants.go b/fuchsia/constants.go
new file mode 100644
index 0000000..ec75826
--- /dev/null
+++ b/fuchsia/constants.go
@@ -0,0 +1,17 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// The file contains Fuchsia-specific constants.
+
+package fuchsia
+
+const (
+	// PlatformManifestName is the name of the manifest of available test
+	// platforms. This file is produced by the build.
+	PlatformManifestName = "platforms.json"
+
+	// PackageManifestName is the name of the manifest of packages included
+	// in a given build. This file is produced by the build.
+	PackageManifestName = "packages.json"
+)
diff --git a/fuchsia/pkg_manifest.go b/fuchsia/pkg_manifest.go
index 5ec7048..6ed3f5b 100644
--- a/fuchsia/pkg_manifest.go
+++ b/fuchsia/pkg_manifest.go
@@ -3,15 +3,14 @@
 // found in the LICENSE file.
 package fuchsia
 
-var (
-	// PackagesManifestFilename represents the name of the packages
-	// manifest file available in the Fuchsia build output directory.
-	PackagesManifestFilename = "packages.json"
+import (
+	"encoding/json"
+	"os"
 )
 
-// PackagesManifest is a JSON structure that's produced by a Fuchsia build and
+// PackageManifest is a JSON structure that's produced by a Fuchsia build and
 // describes which packages were built and where they're located.
-type PackagesManifest struct {
+type PackageManifest struct {
 	Available  []string `json:"available"`
 	Monolith   []string `json:"monolith"`
 	Preinstall []string `json:"preinstall"`
@@ -31,3 +30,16 @@
 	// Name is the basename of the package.
 	Name string `json:"name"`
 }
+
+// LoadPackageManifest loads a PackageManifest specified in JSON at a given
+// filepath.
+func LoadPackageManifest(pkgManifestPath string) (*PackageManifest, error) {
+	manifestFile, err := os.Open(pkgManifestPath)
+	if err != nil {
+		return nil, err
+	}
+	defer manifestFile.Close()
+	var manifest PackageManifest
+	err = json.NewDecoder(manifestFile).Decode(&manifest)
+	return &manifest, err
+}
diff --git a/fuchsia/testexec/doc.go b/fuchsia/testexec/doc.go
new file mode 100644
index 0000000..23025ae
--- /dev/null
+++ b/fuchsia/testexec/doc.go
@@ -0,0 +1,14 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Test authors in the Fuchsia source will specify `environments`in GN within
+// the packages they define their tests. These specifications will be printed
+// to disk in JSON-form during a build.
+//
+// This package is concerned with reading in those specifications, validating
+// that they correspond to valid test environments supported by the
+// infrastructure, and ultimately sharding the associated tests along the lines
+// of those environments.
+
+package testexec
diff --git a/fuchsia/testexec/environment.go b/fuchsia/testexec/environment.go
new file mode 100644
index 0000000..26e7cbe
--- /dev/null
+++ b/fuchsia/testexec/environment.go
@@ -0,0 +1,68 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// We say that an Environment (or FooSpec) "resolves to" another if the
+// properties or constraints given by the former are also given by the latter.
+// This gives a partial ordering on environments expressing a subobject
+// relationship.
+//
+// The utility of being able to express that one environment is a
+// subenvironment of another lies in validation: an environment is valid if
+// it resolves to any member of a golden set of environments we refer to as
+// "platforms", as they correspond to the currently available test platforms
+// supported by the infrastructure.
+//
+// The GN environments specified by test authors in the Fuchsia source
+// correspond directly to the Environment struct defined here.
+package testexec
+
+import (
+	"encoding/json"
+	"io/ioutil"
+)
+
+// Environment describes the environment a test requires.
+type Environment struct {
+	// Device represents properties of the device that are part of the test
+	// environment.
+	Device DeviceSpec `json:"device"`
+}
+
+// ResolvesTo gives a partial ordering on environments in which one environment
+// resolves to another if the properties given by the former are also given by
+// the latter.
+func (env Environment) resolvesTo(other Environment) bool {
+	if !env.Device.resolvesTo(other.Device) {
+		return false
+	}
+	return true
+}
+
+// DeviceSpec describes the device environment a test requires.
+type DeviceSpec struct {
+	// Type represents the class of device the test should run on.
+	// This is a required field.
+	Type string `json:"type"`
+}
+
+func (ds DeviceSpec) resolvesTo(other DeviceSpec) bool {
+	if ds.Type != "" && ds.Type != other.Type {
+		return false
+	}
+	return true
+}
+
+// LoadPlatforms loads the list of test platform environments specified as a
+// JSON list at a given filepath.
+func LoadPlatforms(platformManifestPath string) ([]Environment, error) {
+	bytes, err := ioutil.ReadFile(platformManifestPath)
+	if err != nil {
+		return nil, err
+	}
+	var platforms []Environment
+	if err = json.Unmarshal(bytes, &platforms); err != nil {
+		return nil, err
+	}
+	return platforms, err
+}
diff --git a/fuchsia/testexec/test_platforms.go b/fuchsia/testexec/test_platforms.go
deleted file mode 100644
index 1406cfe..0000000
--- a/fuchsia/testexec/test_platforms.go
+++ /dev/null
@@ -1,45 +0,0 @@
-// Copyright 2018 The Fuchsia Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-package testexec
-
-import (
-	"fmt"
-)
-
-// TestDevicePlatforms is a wrapper type for a set of platforms used for validation of a
-// TestSuiteSpec and its internal structures.
-type TestDevicePlatforms []TestDevicePlatform
-
-// Get retrieves a platform by name.
-func (t TestDevicePlatforms) Get(name string) (*TestDevicePlatform, bool) {
-	for _, platform := range t {
-		if name == platform.Name {
-			return &platform, true
-		}
-	}
-	return nil, false
-}
-
-// Validate validates a given TestDevicePlatforms.
-func (t TestDevicePlatforms) Validate() error {
-	names := map[string]struct{}{}
-	for _, platform := range t {
-		if _, ok := names[platform.Name]; ok {
-			return fmt.Errorf("found duplicate platform %q", platform.Name)
-		}
-		names[platform.Name] = struct{}{}
-	}
-	return nil
-}
-
-// TestDevicePlatform represents a valid platform used for validation of a TestSuiteSpec and its
-// internal structures.
-type TestDevicePlatform struct {
-	// Name is the name of the test device platform.
-	Name string `json:"name"`
-
-	// Arch is the CPU architecture of the test platform.
-	// The value "*" is used as a catch-all for architectures.
-	Arch string `json:"arch"`
-}
diff --git a/fuchsia/testexec/test_spec.go b/fuchsia/testexec/test_spec.go
index 02517a0..94e6b48 100644
--- a/fuchsia/testexec/test_spec.go
+++ b/fuchsia/testexec/test_spec.go
@@ -13,6 +13,24 @@
 	"fuchsia.googlesource.com/infra/infra/fuchsia"
 )
 
+// These are the default environments to be used if an author specifies none.
+//
+// TODO(IN-497) We include the NUC here as a tranistional policy. We will give
+// people a week from flag day to begin explicitly specifying their
+// environments, after which we will set the default as only QEMU.
+var defaultEnvs = []Environment{
+	Environment{
+		Device: DeviceSpec{
+			Type: "QEMU",
+		},
+	},
+	Environment{
+		Device: DeviceSpec{
+			Type: "Intel NUC Kit NUC7i5DNHE",
+		},
+	},
+}
+
 // TestSpec is the specification for a single test and the environments it
 // should be executed in.
 type TestSpec struct {
@@ -23,80 +41,60 @@
 	Envs []Environment `json:"environments"`
 }
 
-// Validate performs basic schema validation on the TestSpec and returns
-// an error if validation fails.
-
-//
-// This should be performed after decoding and before usage to ensure the
-// invariants of the structure are upheld.
-//
-// TODO(mknyszek): Consider making this an actual JSON schema and validate against that.
-// TODO(mknyszek): Don't validate platform policy here once we have it implemented
-// for the input to this tool.
-func (c *TestSpec) Validate(testArch string, platforms TestDevicePlatforms) error {
-	for _, env := range c.Envs {
-		if err := env.Validate(testArch, platforms); err != nil {
-			return err
-		}
-	}
-	return c.Test.Validate()
-}
-
 // Test encapsulates details about a particular test.
 type Test struct {
-	// Location must be a reference to a test. One example of a reference to a test
-	// is a filesystem path. Another could be a Fuchsia URI.
-	// This is a required field.
+	// Name is the full, GN source-relative target name of the test
+	// (e.g., //garnet/bin/foo/tests:foo_tests).
+	Name string `json:"name"`
+
+	// Location is a unique reference to a test: for example, a filesystem
+	// path or a Fuchsia URI.
 	Location string `json:"location"`
 }
 
-// Validate performs basic schema validation on the Test.
-//
-// This should be performed after decoding and before usage to ensure the
-// invariants of the structure are upheld.
-func (t *Test) Validate() error {
-	if t.Location == "" {
-		return fmt.Errorf("Test must have non-empty location")
+func (spec TestSpec) validateAgainst(platforms []Environment) error {
+	if spec.Test.Name == "" {
+		return fmt.Errorf("A test spec's test must have a non-empty name")
+	}
+	if spec.Test.Location == "" {
+		return fmt.Errorf("A test spec's test must have a non-empty location")
+	}
+
+	resolvesToOneOf := func(env Environment, platforms []Environment) bool {
+		for _, platform := range platforms {
+			if env.resolvesTo(platform) {
+				return true
+			}
+		}
+		return false
+	}
+
+	var badEnvs []Environment
+	for _, env := range spec.Envs {
+		if !resolvesToOneOf(env, platforms) {
+			badEnvs = append(badEnvs, env)
+		}
+	}
+	if len(badEnvs) > 0 {
+		return fmt.Errorf(
+			`the following environments of test\n%+v were malformed
+			or did not match any available test platforms:\n%+v`,
+			spec.Test, badEnvs)
 	}
 	return nil
 }
 
-// Environment describes the full environment a test requires.
-//
-// This type must contain only types that are allowed to be used as map keys (i.e.
-// all types must be comparable, https://golang.org/ref/spec#Comparison_operators).
-type Environment struct {
-	// Device represents properties of the device that are part of the test
-	// environment.
-	Device DeviceSpec `json:"device"`
-}
-
-// Validate performs basic schema validation on the Environment.
-func (e *Environment) Validate(testArch string, platforms TestDevicePlatforms) error {
-	return e.Device.Validate(testArch, platforms)
-}
-
-// DeviceSpec describes the device environment a test requires.
-//
-// This type must contain only types that are allowed to be used as map keys (i.e.
-// all types must be comparable, https://golang.org/ref/spec#Comparison_operators).
-type DeviceSpec struct {
-	// Type represents the class of device the test should run on.
-	// This is a required field.
-	Type string `json:"type"`
-}
-
-// Validate performs basic schema validation on the DeviceSpec.
-//
-// This should be performed after decoding and before usage to ensure the
-// invariants of the structure are upheld.
-func (ds *DeviceSpec) Validate(testArch string, platforms TestDevicePlatforms) error {
-	platform, ok := platforms.Get(ds.Type)
-	if !ok {
-		return fmt.Errorf("unknown device type %q", ds.Type)
+// ValidateTestSpecs validates a list of test specs against a list of test
+// platform environments.
+func ValidateTestSpecs(specs []TestSpec, platforms []Environment) error {
+	errMsg := ""
+	for _, spec := range specs {
+		if err := spec.validateAgainst(platforms); err != nil {
+			errMsg += fmt.Sprintf("\n%v", err)
+		}
 	}
-	if platform.Arch != "*" && testArch != platform.Arch {
-		return fmt.Errorf("invalid arch %q for device type %q found", testArch, ds.Type)
+	if errMsg != "" {
+		return fmt.Errorf(errMsg)
 	}
 	return nil
 }
@@ -112,24 +110,8 @@
 	return true
 }
 
-// TODO(IN-673): Add unit tests for the following function.
-// LoadTestSpecs loads a set of test specifications from a manifest of Fuchsia packages
-// produced by a Fuchsia build.
-func LoadTestSpecs(fuchsiaBuildDir string) ([]TestSpec, error) {
-	manifestPath := filepath.Join(fuchsiaBuildDir, fuchsia.PackagesManifestFilename)
-
-	// Read the package manifest produced by the build to find where packages
-	// live in the build directory.
-	manifestFile, err := os.Open(manifestPath)
-	if err != nil {
-		return nil, fmt.Errorf("failed to open packages manifest %s: %s", manifestPath, err.Error())
-	}
-	defer manifestFile.Close()
-	var manifest fuchsia.PackagesManifest
-	if err := json.NewDecoder(manifestFile).Decode(&manifest); err != nil {
-		return nil, fmt.Errorf("failed to parse package manifest %s: %s", manifestPath, err.Error())
-	}
-
+// LoadTestSpecs loads a set of test specifications from a manifest of Fuchsia packages.
+func LoadTestSpecs(fuchsiaBuildDir string, pkgManifest fuchsia.PackageManifest) ([]TestSpec, error) {
 	// Use the packages manifest to discover test specifications.
 	//
 	// It is guaranteed that a test spec will be written to the build directory of the
@@ -137,8 +119,8 @@
 	// <target_out_dir of the test package>/<test package name>.
 	// This algorithm iterates over these directories for each package in the package
 	// manifest.
-	specs := make([]TestSpec, 0, len(manifest.Packages))
-	for _, pkg := range manifest.Packages {
+	specs := make([]TestSpec, 0, len(pkgManifest.Packages))
+	for _, pkg := range pkgManifest.Packages {
 		testSpecDir := filepath.Join(fuchsiaBuildDir, pkg.BuildDir, pkg.Name)
 
 		// If the associated test spec directory does not exist, the package specified no
@@ -177,6 +159,9 @@
 			if err := json.NewDecoder(f).Decode(&spec); err != nil {
 				return nil, fmt.Errorf("failed to decode %s: %s", path, err.Error())
 			}
+			if len(spec.Envs) == 0 {
+				spec.Envs = defaultEnvs
+			}
 			specs = append(specs, spec)
 		}
 	}
diff --git a/fuchsia/testexec/test_spec_test.go b/fuchsia/testexec/test_spec_test.go
new file mode 100644
index 0000000..6a4288a
--- /dev/null
+++ b/fuchsia/testexec/test_spec_test.go
@@ -0,0 +1,266 @@
+// Copyright 2018 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+package testexec_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"reflect"
+	"sort"
+	"testing"
+
+	"fuchsia.googlesource.com/infra/infra/fuchsia"
+	"fuchsia.googlesource.com/infra/infra/fuchsia/testexec"
+)
+
+var qemuEnv = testexec.Environment{
+	Device: testexec.DeviceSpec{
+		Type: "QEMU",
+	},
+}
+
+var nucEnv = testexec.Environment{
+	Device: testexec.DeviceSpec{
+		Type: "NUC",
+	},
+}
+
+var specFoo1 = testexec.TestSpec{
+	Test: testexec.Test{
+		Name:     "//obsidian/bin/foo:foo_unittests",
+		Location: "/system/test/foo_unittests",
+	},
+	Envs: []testexec.Environment{qemuEnv},
+}
+
+var specFoo2 = testexec.TestSpec{
+	Test: testexec.Test{
+		Name:     "//obsidian/bin/foo:foo_integration_tests",
+		Location: "/system/test/foo_integration_tests",
+	},
+	Envs: []testexec.Environment{qemuEnv, nucEnv},
+}
+
+var specBar = testexec.TestSpec{
+	Test: testexec.Test{
+		Name:     "//obsidian/lib/bar:bar_tests",
+		Location: "/system/test/bar_tests",
+	},
+	Envs: []testexec.Environment{qemuEnv},
+}
+
+// FuchsiaBuildDir is a struct representing the root build directory of a fuchsia
+// checkout.
+type fuchsiaBuildDir struct {
+	root string
+	t    *testing.T
+}
+
+func newFuchsiaBuildDir(t *testing.T) *fuchsiaBuildDir {
+	root, err := ioutil.TempDir("", "fuchsia-build-dir")
+	if err != nil {
+		t.Fatalf("could not create fuchsia build directory: %v", err)
+	}
+	return &fuchsiaBuildDir{
+		root: root,
+		t:    t,
+	}
+}
+
+// CreatePackageLayout takes a package manifest and creates the associated
+// directory layout.
+func (bd fuchsiaBuildDir) createPackageLayout(pkgManifest fuchsia.PackageManifest) {
+	for _, pkg := range pkgManifest.Packages {
+		specDir := filepath.Join(bd.root, pkg.BuildDir, pkg.Name)
+		if err := os.MkdirAll(specDir, os.ModePerm); err != nil {
+			bd.t.Fatalf("could not create test spec directory for package \"%s\": %v", pkg.Name, err)
+		}
+	}
+}
+
+func writeTestSpec(t *testing.T, spec testexec.TestSpec, path string) {
+	bytes, err := json.Marshal(&spec)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err = ioutil.WriteFile(path, bytes, os.ModePerm); err != nil {
+		t.Fatalf("could not write test spec to %s: %v", path, err)
+	}
+}
+
+func TestLoadTestSpecs(t *testing.T) {
+	areEqual := func(a, b []testexec.TestSpec) bool {
+		stringify := func(spec testexec.TestSpec) string {
+			return fmt.Sprintf("%#v", spec)
+		}
+		sort := func(list []testexec.TestSpec) {
+			sort.Slice(list[:], func(i, j int) bool {
+				return stringify(list[i]) < stringify(list[j])
+			})
+		}
+		sort(a)
+		sort(b)
+		return reflect.DeepEqual(a, b)
+	}
+
+	packageFoo := "foo"
+	buildDirFoo := "obj/obsidian/bin/foo"
+	packageBar := "bar"
+	buildDirBar := "obj/obsidian/lib/bar"
+
+	pkgManifest := fuchsia.PackageManifest{
+		Packages: []fuchsia.PackageDescriptor{
+			{
+				BuildDir: buildDirFoo,
+				Name:     packageFoo,
+			},
+			{
+				BuildDir: buildDirBar,
+				Name:     packageBar,
+			},
+		},
+	}
+
+	correctSpecsLoad := func(t *testing.T, expected []testexec.TestSpec, fuchsiaBuildDir string) {
+		actual, err := testexec.LoadTestSpecs(fuchsiaBuildDir, pkgManifest)
+		if err != nil {
+			t.Fatalf("error while loading test specs: %v", err)
+		}
+		if !areEqual(expected, actual) {
+			t.Fatalf("test specs not properly loaded:\nexpected:\n%+v\nactual:\n%+v", expected, actual)
+		}
+	}
+
+	t.Run("test specs are found", func(t *testing.T) {
+		bd := newFuchsiaBuildDir(t)
+		defer os.RemoveAll(bd.root)
+		bd.createPackageLayout(pkgManifest)
+
+		specDirFoo := filepath.Join(bd.root, buildDirFoo, packageFoo)
+		specDirBar := filepath.Join(bd.root, buildDirBar, packageBar)
+
+		writeTestSpec(t, specFoo1, filepath.Join(specDirFoo, "foo_unittests.spec.json"))
+		writeTestSpec(t, specFoo2, filepath.Join(specDirFoo, "foo_integration_tests.spec.json"))
+		writeTestSpec(t, specBar, filepath.Join(specDirBar, "bar_tests.spec.json"))
+
+		expected := []testexec.TestSpec{specFoo1, specFoo2, specBar}
+		correctSpecsLoad(t, expected, bd.root)
+	})
+
+	t.Run("test specs in wrong location are ignored", func(t *testing.T) {
+		bd := newFuchsiaBuildDir(t)
+		defer os.RemoveAll(bd.root)
+		bd.createPackageLayout(pkgManifest)
+
+		specDirFoo := filepath.Join(bd.root, buildDirFoo, packageFoo)
+		specDirBar := filepath.Join(bd.root, buildDirBar, packageBar)
+		nonSpecDir := filepath.Join(bd.root, buildDirBar, "other-package")
+		if err := os.MkdirAll(nonSpecDir, os.ModePerm); err != nil {
+			t.Fatalf("failed to create a directory outside of the package manifest: %v", err)
+		}
+
+		writeTestSpec(t, specFoo1, filepath.Join(specDirFoo, "foo_unittests.spec.json"))
+		writeTestSpec(t, specFoo2, filepath.Join(nonSpecDir, "other_tests.spec.json"))
+		writeTestSpec(t, specBar, filepath.Join(specDirBar, "bar_tests.spec.json"))
+
+		expected := []testexec.TestSpec{specFoo1, specBar}
+		correctSpecsLoad(t, expected, bd.root)
+	})
+
+	t.Run("test specs with wrong extension are ignored", func(t *testing.T) {
+		bd := newFuchsiaBuildDir(t)
+		defer os.RemoveAll(bd.root)
+		bd.createPackageLayout(pkgManifest)
+
+		specDirFoo := filepath.Join(bd.root, buildDirFoo, packageFoo)
+		specDirBar := filepath.Join(bd.root, buildDirBar, packageBar)
+
+		writeTestSpec(t, specFoo1, filepath.Join(specDirFoo, "bad_extension1.json"))
+		writeTestSpec(t, specFoo2, filepath.Join(specDirFoo, "good extension.spec.json"))
+		writeTestSpec(t, specBar, filepath.Join(specDirBar, "bad_extension2.spec"))
+
+		expected := []testexec.TestSpec{specFoo2}
+		correctSpecsLoad(t, expected, bd.root)
+	})
+
+	t.Run("malformed test specs raise error", func(t *testing.T) {
+		bd := newFuchsiaBuildDir(t)
+		defer os.RemoveAll(bd.root)
+		bd.createPackageLayout(pkgManifest)
+
+		specDirFoo := filepath.Join(bd.root, buildDirFoo, packageFoo)
+		specDirBar := filepath.Join(bd.root, buildDirBar, packageBar)
+
+		writeTestSpec(t, specFoo1, filepath.Join(specDirFoo, "foo_unittests.spec.json"))
+		if err := ioutil.WriteFile(filepath.Join(specDirFoo, "foo_integration_tests.spec.json"),
+			[]byte("{I am not a test spec}"), os.ModePerm); err != nil {
+			t.Fatalf("could not write malformed test spec: %v", err)
+		}
+		writeTestSpec(t, specBar, filepath.Join(specDirBar, "bar_tests.spec.json"))
+
+		_, err := testexec.LoadTestSpecs(bd.root, pkgManifest)
+		if err == nil {
+			t.Fatalf("malformed test spec did not raise an error")
+		}
+	})
+}
+
+func TestValidateTestSpecs(t *testing.T) {
+	noTestNameSpec := testexec.TestSpec{
+		Test: testexec.Test{
+			Location: "/system/test/baz_tests",
+		},
+		Envs: []testexec.Environment{qemuEnv},
+	}
+	noTestLocationSpec := testexec.TestSpec{
+		Test: testexec.Test{
+			Name: "//obsidian/public/lib/baz:baz_tests",
+		},
+		Envs: []testexec.Environment{qemuEnv},
+	}
+	badEnvSpec := testexec.TestSpec{
+		Test: testexec.Test{
+			Name:     "//obsidian/public/lib/baz:baz_tests",
+			Location: "/system/test/baz_tests",
+		},
+		Envs: []testexec.Environment{
+			testexec.Environment{
+				Device: testexec.DeviceSpec{
+					Type: "NON-EXISTENT-DEVICE",
+				},
+			},
+		},
+	}
+	platforms := []testexec.Environment{qemuEnv, nucEnv}
+
+	t.Run("valid specs are validated", func(t *testing.T) {
+		validSpecLists := [][]testexec.TestSpec{
+			{specFoo1}, {specFoo2}, {specBar},
+			{specFoo1, specFoo2}, {specFoo1, specBar}, {specFoo2, specBar},
+			{specFoo1, specFoo2, specBar},
+		}
+		for _, list := range validSpecLists {
+			if err := testexec.ValidateTestSpecs(list, platforms); err != nil {
+				t.Fatalf("valid specs marked as invalid: %+v: %v", list, err)
+			}
+		}
+	})
+
+	t.Run("invalid specs are invalidated", func(t *testing.T) {
+		invalidSpecLists := [][]testexec.TestSpec{
+			{noTestNameSpec}, {noTestLocationSpec}, {badEnvSpec},
+			{noTestNameSpec, noTestLocationSpec}, {noTestNameSpec, badEnvSpec},
+			{noTestLocationSpec, badEnvSpec},
+			{noTestNameSpec, noTestLocationSpec, badEnvSpec},
+		}
+		for _, list := range invalidSpecLists {
+			if err := testexec.ValidateTestSpecs(list, platforms); err == nil {
+				t.Fatalf("invalid specs marked as valid: %+v", list)
+			}
+		}
+	})
+}
diff --git a/fuchsia/testexec/validation_test.go b/fuchsia/testexec/validation_test.go
deleted file mode 100644
index 91d636b..0000000
--- a/fuchsia/testexec/validation_test.go
+++ /dev/null
@@ -1,138 +0,0 @@
-// Copyright 2018 The Fuchsia Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-package testexec
-
-import (
-	"testing"
-)
-
-func TestTestDevicePlatformsValidation(t *testing.T) {
-	t.Run("Ensure names are unique", func(t *testing.T) {
-		platforms := TestDevicePlatforms{
-			{
-				Name: "NUC",
-				Arch: "arm64",
-			},
-			{
-				Name: "NUC",
-				Arch: "x64",
-			},
-		}
-		if err := platforms.Validate(); err == nil {
-			t.Fatal("platforms erroneously passed validation")
-		}
-	})
-}
-
-func TestTestSpecValidation(t *testing.T) {
-	noPlatforms := TestDevicePlatforms{}
-	nucPlatform := TestDevicePlatforms{
-		{
-			Name: "NUC",
-			Arch: "x64",
-		},
-	}
-	allArchPlatform := TestDevicePlatforms{
-		{
-			Name: "QEMU",
-			Arch: "*",
-		},
-	}
-	allPlatforms := TestDevicePlatforms{
-		{
-			Name: "NUC",
-			Arch: "x64",
-		},
-		{
-			Name: "QEMU",
-			Arch: "*",
-		},
-	}
-	t.Run("Ensure error on missing specifier", func(t *testing.T) {
-		spec := TestSpec{Test: Test{}}
-		if err := spec.Validate("x64", noPlatforms); err == nil {
-			t.Fatal("spec erroneously passed validation")
-		}
-	})
-	t.Run("Ensure error on unknown platform", func(t *testing.T) {
-		spec := TestSpec{
-			Test: Test{Location: "/path/to/binary"},
-			Envs: []Environment{
-				{
-					Device: DeviceSpec{
-						Type: "idk friend",
-					},
-				},
-			},
-		}
-		if err := spec.Validate("x64", nucPlatform); err == nil {
-			t.Fatal("spec erroneously passed validation")
-		}
-	})
-	t.Run("Ensure error on unknown arch for platform", func(t *testing.T) {
-		spec := TestSpec{
-			Test: Test{Location: "/path/to/binary"},
-			Envs: []Environment{
-				{
-					Device: DeviceSpec{
-						Type: "NUC",
-					},
-				},
-			},
-		}
-		if err := spec.Validate("arm64", nucPlatform); err == nil {
-			t.Fatal("spec erroneously passed validation")
-		}
-	})
-	t.Run("All arch OK", func(t *testing.T) {
-		spec := TestSpec{
-			Test: Test{Location: "/path/to/binary"},
-			Envs: []Environment{
-				{
-					Device: DeviceSpec{
-						Type: "QEMU",
-					},
-				},
-			},
-		}
-		if err := spec.Validate("mycpuarch", allArchPlatform); err != nil {
-			t.Fatal("good config failed validation:", err)
-		}
-	})
-	t.Run("Basic config OK", func(t *testing.T) {
-		spec := TestSpec{
-			Test: Test{Location: "/path/to/binary"},
-			Envs: []Environment{
-				{
-					Device: DeviceSpec{
-						Type: "NUC",
-					},
-				},
-			},
-		}
-		if err := spec.Validate("x64", nucPlatform); err != nil {
-			t.Fatal("good config failed validation:", err)
-		}
-	})
-	t.Run("Multiple envs OK", func(t *testing.T) {
-		spec := TestSpec{
-			Test: Test{Location: "/path/to/binary"},
-			Envs: []Environment{
-				{
-					Device: DeviceSpec{
-						Type: "NUC",
-					},
-				},
-				{
-					Device: DeviceSpec{
-						Type: "QEMU",
-					},
-				},
-			},
-		}
-		if err := spec.Validate("x64", allPlatforms); err != nil {
-			t.Fatal("good config failed validation:", err)
-		}
-	})
-}