Browse Source

Fix #295

- Better pipeline resolution error handling
- Add the missing feature to ShotnessAnalysis

Signed-off-by: Vadim Markovtsev <vadim@sourced.tech>
Vadim Markovtsev 5 years ago
parent
commit
e8e3aeeb72
4 changed files with 18 additions and 9 deletions
  1. 10 6
      internal/core/pipeline.go
  2. 1 1
      internal/core/pipeline_test.go
  3. 6 2
      leaves/shotness.go
  4. 1 0
      leaves/shotness_test.go

+ 10 - 6
internal/core/pipeline.go

@@ -385,7 +385,7 @@ func (pipeline *Pipeline) DeployItem(item PipelineItem) PipelineItem {
 			pipeline.SetFeature(f)
 		}
 	}
-	queue := []PipelineItem{}
+	var queue []PipelineItem
 	queue = append(queue, item)
 	added := map[string]PipelineItem{}
 	for _, item := range pipeline.items {
@@ -514,7 +514,7 @@ func (items sortablePipelineItems) Swap(i, j int) {
 	items[i], items[j] = items[j], items[i]
 }
 
-func (pipeline *Pipeline) resolve(dumpPath string) {
+func (pipeline *Pipeline) resolve(dumpPath string) error {
 	graph := toposort.NewGraph()
 	sort.Sort(sortablePipelineItems(pipeline.items))
 	name2item := map[string]PipelineItem{}
@@ -553,7 +553,7 @@ func (pipeline *Pipeline) resolve(dumpPath string) {
 						fmt.Fprintln(os.Stderr, "]")
 					}
 					pipeline.l.Critical("Failed to resolve pipeline dependencies: ambiguous graph.")
-					return
+					return errors.New("ambiguous graph")
 				}
 				ambiguousMap[key] = graph.FindParents(key)
 			}
@@ -571,7 +571,7 @@ func (pipeline *Pipeline) resolve(dumpPath string) {
 			key = "[" + key + "]"
 			if graph.AddEdge(key, name) == 0 {
 				pipeline.l.Criticalf("Unsatisfied dependency: %s -> %s", key, item.Name())
-				return
+				return errors.New("unsatisfied dependency")
 			}
 		}
 	}
@@ -625,7 +625,7 @@ func (pipeline *Pipeline) resolve(dumpPath string) {
 	strplan, ok := graph.Toposort()
 	if !ok {
 		pipeline.l.Critical("Failed to resolve pipeline dependencies: unable to topologically sort the items.")
-		return
+		return errors.New("topological sort failure")
 	}
 	pipeline.items = make([]PipelineItem, 0, len(pipeline.items))
 	for _, key := range strplan {
@@ -640,6 +640,7 @@ func (pipeline *Pipeline) resolve(dumpPath string) {
 		absPath, _ := filepath.Abs(dumpPath)
 		pipeline.l.Infof("Wrote the DAG to %s\n", absPath)
 	}
+	return nil
 }
 
 // Initialize prepares the pipeline for the execution (Run()). This function
@@ -685,7 +686,10 @@ func (pipeline *Pipeline) Initialize(facts map[string]interface{}) error {
 		pipeline.HibernationDistance = val
 	}
 	dumpPath, _ := facts[ConfigPipelineDAGPath].(string)
-	pipeline.resolve(dumpPath)
+	err := pipeline.resolve(dumpPath)
+	if err != nil {
+		return err
+	}
 	if dumpPlan, exists := facts[ConfigPipelineDumpPlan].(bool); exists {
 		pipeline.DumpPlan = dumpPlan
 	}

+ 1 - 1
internal/core/pipeline_test.go

@@ -510,7 +510,7 @@ func TestPipelineError(t *testing.T) {
 	item := &testPipelineItem{}
 	item.TestError = true
 	pipeline.AddItem(item)
-	pipeline.Initialize(map[string]interface{}{})
+	assert.NoError(t, pipeline.Initialize(map[string]interface{}{}))
 	commits := make([]*object.Commit, 1)
 	commits[0], _ = test.Repository.CommitObject(plumbing.NewHash(
 		"af9ddc0db70f09f3f27b4b98e415592a7485171c"))

+ 6 - 2
leaves/shotness.go

@@ -92,8 +92,7 @@ func (shotness *ShotnessAnalysis) Provides() []string {
 // Each requested entity will be inserted into `deps` of Consume(). In turn, those
 // entities are Provides() upstream.
 func (shotness *ShotnessAnalysis) Requires() []string {
-	arr := [...]string{items.DependencyFileDiff, uast_items.DependencyUastChanges}
-	return arr[:]
+	return []string{items.DependencyFileDiff, uast_items.DependencyUastChanges}
 }
 
 // ListConfigurationOptions returns the list of changeable public properties of this PipelineItem.
@@ -120,6 +119,11 @@ func (shotness *ShotnessAnalysis) Flag() string {
 	return "shotness"
 }
 
+// Flag returns the command line switch which activates the analysis.
+func (shotness *ShotnessAnalysis) Features() []string {
+	return []string{uast_items.FeatureUast}
+}
+
 // Description returns the text which explains what the analysis is doing.
 func (shotness *ShotnessAnalysis) Description() string {
 	return "Structural hotness - a fine-grained alternative to --couples. " +

+ 1 - 0
leaves/shotness_test.go

@@ -57,6 +57,7 @@ func TestShotnessMeta(t *testing.T) {
 		core.ConfigLogger: logger,
 	}))
 	assert.Equal(t, logger, sh.l)
+	assert.Equal(t, []string{uast_items.FeatureUast}, sh.Features())
 }
 
 func TestShotnessRegistration(t *testing.T) {