Skip to content

Commit 9429e5b

Browse files
tfg13tfg13
committed
Revert "New test runner implementation"
This reverts commit d834c4f. # Conflicts: # pkg/statectx/context.go
1 parent 8275eb4 commit 9429e5b

25 files changed

Lines changed: 1975 additions & 1519 deletions

File tree

‎pkg/cerrors/cerrors.go‎

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,6 @@ func (e *ErrTestStepsNeverReturned) Error() string {
4343
return fmt.Sprintf("test step [%s] did not return", strings.Join(e.StepNames, ", "))
4444
}
4545

46-
// ErrTestTargetInjectionTimedOut indicates that test step did not ingest a target
47-
// within allotted time.
48-
type ErrTestTargetInjectionTimedOut struct {
49-
StepName string
50-
}
51-
52-
// Error returns the error string associated with the error
53-
func (e *ErrTestTargetInjectionTimedOut) Error() string {
54-
return fmt.Sprintf("test step %v failed to ingest a target", e.StepName)
55-
}
56-
5746
// ErrTestStepClosedChannels indicates that the test step returned after
5847
// closing its output channels, which constitutes an API violation
5948
type ErrTestStepClosedChannels struct {
@@ -64,47 +53,3 @@ type ErrTestStepClosedChannels struct {
6453
func (e *ErrTestStepClosedChannels) Error() string {
6554
return fmt.Sprintf("test step %v closed output channels (api violation)", e.StepName)
6655
}
67-
68-
// ErrTestStepPaniced indicates that a test step's method paniced.
69-
type ErrTestStepPaniced struct {
70-
StepName string
71-
StackTrace string
72-
}
73-
74-
// Error returns the error string associated with the error
75-
func (e *ErrTestStepPaniced) Error() string {
76-
return fmt.Sprintf("test step %s paniced, trace: %q", e.StepName, e.StackTrace)
77-
}
78-
79-
// ErrTestStepReturnedDuplicateResult indicates that a test step's method paniced.
80-
type ErrTestStepReturnedDuplicateResult struct {
81-
StepName string
82-
Target string
83-
}
84-
85-
// Error returns the error string associated with the error
86-
func (e *ErrTestStepReturnedDuplicateResult) Error() string {
87-
return fmt.Sprintf("test step %s returned duplicate result for %s", e.StepName, e.Target)
88-
}
89-
90-
// ErrTestStepReturnedUnexpectedResult indicates that a test step's method paniced.
91-
type ErrTestStepReturnedUnexpectedResult struct {
92-
StepName string
93-
Target string
94-
}
95-
96-
// Error returns the error string associated with the error
97-
func (e *ErrTestStepReturnedUnexpectedResult) Error() string {
98-
return fmt.Sprintf("test step %s returned unexpected result for %s", e.StepName, e.Target)
99-
}
100-
101-
// ErrTestStepLostTargets indicates that targets have been lost during test run.
102-
type ErrTestStepLostTargets struct {
103-
StepName string
104-
Target string
105-
}
106-
107-
// Error returns the error string associated with the error
108-
func (e *ErrTestStepLostTargets) Error() string {
109-
return fmt.Sprintf("test step %s lost target %s", e.StepName, e.Target)
110-
}

‎pkg/config/timeouts.go‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,35 @@ var TargetManagerAcquireTimeout = 5 * time.Minute
1616
var TargetManagerReleaseTimeout = 5 * time.Minute
1717

1818
// StepInjectTimeout represents the maximum time that TestRunner will wait for
19-
// a TestStep to accept a Target
19+
// the first TestStep of the pipeline to accept a Target
2020
var StepInjectTimeout = 30 * time.Second
2121

22-
// TestRunnerShutdownTimeout represents the maximum time that the TestRunner
23-
// will wait for all TestSteps to complete after all Targets have reached the end
24-
// of the pipeline.
22+
// TestRunnerMsgTimeout represents the maximum time that any component of the
23+
// TestRunner will wait for the delivery of a message to any other subsystem
24+
// of the TestRunner
25+
var TestRunnerMsgTimeout = 5 * time.Second
26+
27+
// TestRunnerShutdownTimeout represents the maximum time that the TestRunner will
28+
// wait for all the TestStep to complete after a cancellation signal has been
29+
// delivered
30+
31+
// TestRunnerShutdownTimeout controls a block of the TestRunner which works as a
32+
// watchdog, i.e. if there are multiple steps that need to return, the timeout is
33+
// reset every time a step returns. The timeout should be handled so that it
34+
// doesn't reset when a TestStep returns.
2535
var TestRunnerShutdownTimeout = 30 * time.Second
2636

37+
// TestRunnerStepShutdownTimeout represents the maximum time that the TestRunner
38+
// will wait for all TestSteps to complete after all Targets have reached the end
39+
// of the pipeline. This timeout is only relevant if a cancellation signal is *not*
40+
// delivered.
41+
42+
// TestRunnerStepShutdownTimeout controls a block of the TestRunner which worksas
43+
// a watchdog, i.e. if there are multiple steps that need to return, the timeout
44+
// is reset every time a step returns. The timeout should be handled so that it
45+
// doesn't reset when a TestStep returns.
46+
var TestRunnerStepShutdownTimeout = 5 * time.Second
47+
2748
// LockRefreshTimeout is the amount of time by which a target lock is extended
2849
// periodically while a job is running.
2950
var LockRefreshTimeout = 1 * time.Minute

‎pkg/event/testevent/test.go‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
"github.com/facebookincubator/contest/pkg/types"
1717
)
1818

19-
// Header models the header of a test event, which consists in metadata that defines the
19+
// Header models the header of a test event, which consists in metadatat hat defines the
2020
// emitter of the events. The Header is under ConTest control and cannot be manipulated
2121
// by the TestStep
2222
type Header struct {
@@ -28,8 +28,8 @@ type Header struct {
2828

2929
// Data models the data of a test event. It is populated by the TestStep
3030
type Data struct {
31-
Target *target.Target
3231
EventName event.Name
32+
Target *target.Target
3333
Payload *json.RawMessage
3434
}
3535

@@ -152,15 +152,3 @@ type EmitterFetcher interface {
152152
Emitter
153153
Fetcher
154154
}
155-
156-
func (h *Header) String() string {
157-
return fmt.Sprintf("[%d %d %s %s]", h.JobID, h.RunID, h.TestName, h.TestStepLabel)
158-
}
159-
160-
func (d *Data) String() string {
161-
ps := ""
162-
if d.Payload != nil {
163-
ps = fmt.Sprintf(" %q", d.Payload) //nolint SA5009 - works fine
164-
}
165-
return fmt.Sprintf("[%s %s%s]", d.Target, d.EventName, ps)
166-
}

‎pkg/jobmanager/jobmanager.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ func newPartialJobFromDescriptor(pr *pluginregistry.PluginRegistry, jd *job.JobD
161161
}
162162
// test step index is incremented by 1 so we can use 0 to signal an
163163
// anomaly.
164-
tsb, err := pr.NewTestStepBundle(*testStepDesc, tse)
164+
tsb, err := pr.NewTestStepBundle(*testStepDesc, uint(idx)+1, tse)
165165
if err != nil {
166166
return nil, fmt.Errorf("NewTestStepBundle for test step '%s' with index %d failed: %w", testStepDesc.Name, idx, err)
167167
}

‎pkg/pluginregistry/bundles.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
)
1616

1717
// NewTestStepBundle creates a TestStepBundle from a TestStepDescriptor
18-
func (r *PluginRegistry) NewTestStepBundle(testStepDescriptor test.TestStepDescriptor, allowedEvents map[event.Name]bool) (*test.TestStepBundle, error) {
18+
func (r *PluginRegistry) NewTestStepBundle(testStepDescriptor test.TestStepDescriptor, stepIndex uint, allowedEvents map[event.Name]bool) (*test.TestStepBundle, error) {
1919
testStep, err := r.NewTestStep(testStepDescriptor.Name)
2020
if err != nil {
2121
return nil, fmt.Errorf("could not get the desired TestStep (%s): %v", testStepDescriptor.Name, err)

‎pkg/runner/job_runner.go‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"github.com/facebookincubator/contest/pkg/event/testevent"
1818
"github.com/facebookincubator/contest/pkg/job"
1919
"github.com/facebookincubator/contest/pkg/logging"
20-
"github.com/facebookincubator/contest/pkg/statectx"
2120
"github.com/facebookincubator/contest/pkg/storage"
2221
"github.com/facebookincubator/contest/pkg/target"
2322
"github.com/facebookincubator/contest/pkg/types"
@@ -158,8 +157,9 @@ func (jr *JobRunner) Run(j *job.Job) ([][]*job.Report, []*job.Report, error) {
158157
if err := tl.Unlock(j.ID, targets); err != nil {
159158
jobLog.Warningf("Failed to unlock targets (%v) for job ID %d: %v", targets, j.ID, err)
160159
}
160+
case <-j.StateCtx.Paused():
161+
jobLog.Debugf("Received pause request, NOT releasing targets so the job can be resumed")
161162
return
162-
// Ignore the pause signal, continue to refresh targets.
163163
case <-done:
164164
if err := tl.Unlock(j.ID, targets); err != nil {
165165
jobLog.Warningf("Failed to unlock %d target(s) (%v): %v", len(targets), targets, err)
@@ -183,14 +183,7 @@ func (jr *JobRunner) Run(j *job.Job) ([][]*job.Report, []*job.Report, error) {
183183
if runErr = jr.emitAcquiredTargets(testEventEmitter, targets); runErr == nil {
184184
jobLog.Infof("Run #%d: running test #%d for job '%s' (job ID: %d) on %d targets", run+1, idx, j.Name, j.ID, len(targets))
185185
testRunner := NewTestRunner()
186-
var resumeState []byte
187-
resumeState, err := testRunner.Run(j.StateCtx, t, targets, j.ID, types.RunID(run+1), resumeState)
188-
if err == statectx.ErrPaused {
189-
jobLog.Debugf("Runner paused, state: %s", string(resumeState))
190-
// TODO(rojer): Persist the state.
191-
} else {
192-
runErr = err
193-
}
186+
runErr = testRunner.Run(j.StateCtx, t, targets, j.ID, types.RunID(run+1))
194187
}
195188

196189
// Job is done, release all the targets

0 commit comments

Comments
 (0)