Address review comments

facebookarchive · Jan 18, 2021 · 86b113d · 86b113d
1 parent 5c93032
commit 86b113d
Show file tree

Hide file tree

Showing 7 changed files with 214 additions and 128 deletions.
diff --git a/pkg/cerrors/cerrors.go b/pkg/cerrors/cerrors.go
@@ -65,7 +65,7 @@ func (e *ErrTestStepClosedChannels) Error() string {
 	return fmt.Sprintf("test step %v closed output channels (api violation)", e.StepName)
 }
 
-// ErrTestStepPaniced indicates that a test step's method paniced.
+// ErrTestStepPaniced indicates that a test step's method panicked.
 type ErrTestStepPaniced struct {
 	StepName   string
 	StackTrace string
@@ -76,7 +76,8 @@ func (e *ErrTestStepPaniced) Error() string {
 	return fmt.Sprintf("test step %s paniced, trace: %q", e.StepName, e.StackTrace)
 }
 
-// ErrTestStepReturnedDuplicateResult indicates that a test step's method paniced.
+// ErrTestStepReturnedDuplicateResult indicates that a test step returned result
+// twice for the same target.
 type ErrTestStepReturnedDuplicateResult struct {
 	StepName string
 	Target   string
@@ -87,7 +88,8 @@ func (e *ErrTestStepReturnedDuplicateResult) Error() string {
 	return fmt.Sprintf("test step %s returned duplicate result for %s", e.StepName, e.Target)
 }
 
-// ErrTestStepReturnedUnexpectedResult indicates that a test step's method paniced.
+// ErrTestStepReturnedUnexpectedResult indicates that a test step returned result
+// for a target that was not given to it.
 type ErrTestStepReturnedUnexpectedResult struct {
 	StepName string
 	Target   string

diff --git a/pkg/jobmanager/jobmanager.go b/pkg/jobmanager/jobmanager.go
@@ -158,8 +158,6 @@ func newPartialJobFromDescriptor(pr *pluginregistry.PluginRegistry, jd *job.JobD
 			if err != nil {
 				return nil, err
 			}
-			// test step index is incremented by 1 so we can use 0 to signal an
-			// anomaly.
 			tsb, err := pr.NewTestStepBundle(*testStepDesc, tse)
 			if err != nil {
 				return nil, fmt.Errorf("NewTestStepBundle for test step '%s' with index %d failed: %w", testStepDesc.Name, idx, err)

diff --git a/pkg/runner/job_runner.go b/pkg/runner/job_runner.go
@@ -186,7 +186,7 @@ func (jr *JobRunner) Run(j *job.Job) ([][]*job.Report, []*job.Report, error) {
 				var resumeState []byte
 				resumeState, err := testRunner.Run(j.StateCtx, t, targets, j.ID, types.RunID(run+1), resumeState)
 				if err == statectx.ErrPaused {
-					jobLog.Debugf("Runner paused, state: %s", string(resumeState))
+					jobLog.Errorf("Runner paused, state: %s", string(resumeState))
 					// TODO(rojer): Persist the state.
 				} else {
 					runErr = err