Merge pull request #56 from linuxkit/fix-run-sharding

simplify shard algorithm to fix

Author: deitch

cmd/list.go

@@ -34,7 +34,7 @@ func init() {
 	flags := listCmd.Flags()
 	// shardPattern is 1-based (1/10, 3/10, 10/10) rather than normal computer 0-based (0/9, 2/9, 9/9), because it is easier for
 	// humans to understand when calling the CLI.
-	flags.StringVarP(&shardPattern, "shard", "s", "", "which shard to run, in form of 'N/M' where N is the shard number and M is the total number of shards, smallest shard number is 1")
+	flags.StringVarP(&shardPattern, "shard", "s", "", "which shard to run, in form of 'N/M' where N is the shard number and M is the total number of shards, smallest shard number is 1. Shards are applied only to tests that would run, not those that would be skipped.")
 	RootCmd.AddCommand(listCmd)
 }
 

cmd/run.go

@@ -93,7 +93,7 @@ func init() {
 	flags.BoolVarP(&parallel, "parallel", "p", false, "Run multiple tests in parallel")
 	// shardPattern is 1-based (1/10, 3/10, 10/10) rather than normal computer 0-based (0/9, 2/9, 9/9), because it is easier for
 	// humans to understand when calling the CLI.
-	flags.StringVarP(&shardPattern, "shard", "s", "", "which shard to run, in form of 'N/M' where N is the shard number and M is the total number of shards, smallest shard number is 1")
+	flags.StringVarP(&shardPattern, "shard", "s", "", "which shard to run, in form of 'N/M' where N is the shard number and M is the total number of shards, smallest shard number is 1. Shards are applied only to tests that would run, not those that would be skipped.")
 	RootCmd.AddCommand(runCmd)
 }
 

local/group.go

@@ -128,7 +128,7 @@ func (g *Group) List(config RunConfig) []Info {
 }
 
 // Gather gathers all runnable child groups and tests
-func (g *Group) Gather(config RunConfig, count int) ([]TestContainer, int) {
+func (g *Group) Gather(config RunConfig) ([]TestContainer, int) {
 	sort.Sort(ByOrder(g.Children))
 
 	if !g.willRun(config) {
@@ -142,7 +142,7 @@ func (g *Group) Gather(config RunConfig, count int) ([]TestContainer, int) {
 	}
 
 	for _, c := range g.Children {
-		lst, childCount := c.Gather(config, count+subCount)
+		lst, childCount := c.Gather(config)
 		// if we had no runnable tests, do not bother adding the group init/deinit, just return the empty list
 		if childCount == 0 {
 			continue
@@ -165,8 +165,7 @@ func (g *Group) Run(config RunConfig) ([]Result, error) {
 	// This gathers all of the individual tests and group init/deinit commands
 	// all the way down, leading to a flat list we can execute, rather than recursion.
 	// That should make it easier to break into shards.
-	count := 0
-	runnables, _ := g.Gather(config, count)
+	runnables, _ := g.Gather(config)
 	if len(runnables) == 0 {
 		return []Result{{TestResult: Skip,
 			Name:      g.Name(),
@@ -184,21 +183,11 @@ func (g *Group) Run(config RunConfig) ([]Result, error) {
 			wg.Add(1)
 			go func(c TestContainer, cf RunConfig) {
 				defer wg.Done()
-				var isTest bool
-				if _, ok := c.(*Test); ok {
-					isTest = true
-				}
-				// Run() if one of the following is true: it is not a test; there are no start/end boundaries; the test is within the boundaries
-				if !isTest || (config.start == 0 && config.count == 0) || (count >= config.start && config.start+config.count > count) {
-					res, err := c.Run(cf)
-					if err != nil {
-						errCh <- err
-					}
-					resCh <- res
-				}
-				if isTest {
-					count++
+				res, err := c.Run(cf)
+				if err != nil {
+					errCh <- err
 				}
+				resCh <- res
 			}(c, config)
 		}
 

local/groupcommand.go

@@ -38,6 +38,6 @@ func (g GroupCommand) Order() int {
 }
 
 // Gather satisfies the TestContainer interface
-func (g GroupCommand) Gather(config RunConfig, count int) ([]TestContainer, int) {
+func (g GroupCommand) Gather(config RunConfig) ([]TestContainer, int) {
 	return []TestContainer{&g}, 0
 }

local/project.go

@@ -4,6 +4,8 @@ import (
 	"errors"
 	"fmt"
 	"path/filepath"
+
+	"github.com/linuxkit/rtf/logger"
 )
 
 // NewProject creates a new top-level Group at the provided path
@@ -52,11 +54,12 @@ func (p *Project) Run(config RunConfig) ([]Result, error) {
 		return p.Group.Run(config)
 	}
 
-	infos := p.Group.List(config)
-	start, count := calculateShard(len(infos), p.shard, p.totalShards)
-
-	config.start = start
-	config.count = count
+	infos := p.List(config)
+	config.restrictToTests = map[string]bool{}
+	for _, info := range infos {
+		config.restrictToTests[info.Name] = true
+	}
+	config.Logger.Log(logger.LevelInfo, fmt.Sprintf("restricting tests to %v", config.restrictToTests))
 
 	return p.Group.Run(config)
 }
@@ -67,6 +70,14 @@ func (p *Project) List(config RunConfig) []Info {
 	if p.totalShards <= 1 {
 		return infos
 	}
-	start, count := calculateShard(len(infos), p.shard, p.totalShards)
-	return infos[start : start+count]
+	// if sharding, we ignore tests that would be skipped
+	var retInfos []Info
+	for _, info := range infos {
+		if info.TestResult == Skip {
+			continue
+		}
+		retInfos = append(retInfos, info)
+	}
+	start, count := calculateShard(len(retInfos), p.shard, p.totalShards)
+	return retInfos[start : start+count]
 }

local/test.go

@@ -82,8 +82,8 @@ func (t *Test) List(config RunConfig) []Info {
 }
 
 // Gather satisfies the TestContainer interface
-func (t *Test) Gather(config RunConfig, count int) ([]TestContainer, int) {
-	if (config.start == 0 && config.count == 0) || (count >= config.start && config.start+config.count > count) {
+func (t *Test) Gather(config RunConfig) ([]TestContainer, int) {
+	if len(config.restrictToTests) == 0 || config.restrictToTests[t.Name()] {
 		return []TestContainer{t}, 1
 	}
 	return nil, 0
@@ -101,6 +101,7 @@ func (t *Test) Run(config RunConfig) ([]Result, error) {
 			TestResult: Skip,
 		}}, nil
 	}
+	config.Logger.Log(logger.LevelInfo, fmt.Sprintf("Running test %s", t.Name()))
 
 	if t.Tags.Repeat == 0 {
 		// Always run at least once

local/types.go

@@ -160,18 +160,17 @@ type OSInfo struct {
 
 // RunConfig contains runtime configuration information
 type RunConfig struct {
-	Extra       bool
-	CaseDir     string
-	LogDir      string
-	Logger      logger.LogDispatcher
-	SystemInfo  sysinfo.SystemInfo
-	Labels      map[string]bool
-	NotLabels   map[string]bool
-	TestPattern string
-	Parallel    bool
-	IncludeInit bool
-	start       int
-	count       int
+	Extra           bool
+	CaseDir         string
+	LogDir          string
+	Logger          logger.LogDispatcher
+	SystemInfo      sysinfo.SystemInfo
+	Labels          map[string]bool
+	NotLabels       map[string]bool
+	TestPattern     string
+	Parallel        bool
+	IncludeInit     bool
+	restrictToTests map[string]bool
 }
 
 // GroupCommand is a command that is runnable, either a test or pre/post script.
@@ -187,7 +186,7 @@ type TestContainer interface {
 	Order() int
 	List(config RunConfig) []Info
 	Run(config RunConfig) ([]Result, error)
-	Gather(config RunConfig, count int) ([]TestContainer, int)
+	Gather(config RunConfig) ([]TestContainer, int)
 }
 
 // ByOrder implements the sort.Sorter interface for TestContainer