From e8b032035e80f3969683c8dbd0b3a1e59cbc93e2 Mon Sep 17 00:00:00 2001
From: Nedyalko Andreev <n@andreev.sh>
Date: Wed, 7 Dec 2022 13:47:10 +0200
Subject: [PATCH] Move the RunStatus type from the lib/ package to cloudapi/

The run_status constants are something very specific to the k6 cloud, they shouldn't be a part of the main packages.
---
 cloudapi/api.go                        | 14 +++++++-----
 {lib => cloudapi}/run_status.go        |  9 +++-----
 cmd/cloud.go                           |  8 +++----
 cmd/tests/cmd_test.go                  | 31 +++++++++++++-------------
 core/engine.go                         | 15 +++++++------
 lib/testutils/mockoutput/mockoutput.go | 10 ---------
 output/cloud/output.go                 |  8 +++----
 output/cloud/output_test.go            |  4 ++--
 output/manager.go                      |  4 ++--
 output/types.go                        |  3 ++-
 10 files changed, 48 insertions(+), 58 deletions(-)
 rename {lib => cloudapi}/run_status.go (71%)

diff --git a/cloudapi/api.go b/cloudapi/api.go
index f092668221b..9923fee6b8c 100644
--- a/cloudapi/api.go
+++ b/cloudapi/api.go
@@ -34,10 +34,10 @@ type CreateTestRunResponse struct {
 }
 
 type TestProgressResponse struct {
-	RunStatusText string        `json:"run_status_text"`
-	RunStatus     lib.RunStatus `json:"run_status"`
-	ResultStatus  ResultStatus  `json:"result_status"`
-	Progress      float64       `json:"progress"`
+	RunStatusText string       `json:"run_status_text"`
+	RunStatus     RunStatus    `json:"run_status"`
+	ResultStatus  ResultStatus `json:"result_status"`
+	Progress      float64      `json:"progress"`
 }
 
 type LoginResponse struct {
@@ -107,7 +107,9 @@ func (c *Client) StartCloudTestRun(name string, projectID int64, arc *lib.Archiv
 	return ctrr.ReferenceID, nil
 }
 
-func (c *Client) TestFinished(referenceID string, thresholds ThresholdResult, tained bool, runStatus lib.RunStatus) error {
+// TestFinished sends the result and run status values to the cloud, along with
+// information for the test thresholds, and marks the test run as finished.
+func (c *Client) TestFinished(referenceID string, thresholds ThresholdResult, tained bool, runStatus RunStatus) error {
 	url := fmt.Sprintf("%s/tests/%s", c.baseURL, referenceID)
 
 	resultStatus := ResultStatusPassed
@@ -117,7 +119,7 @@ func (c *Client) TestFinished(referenceID string, thresholds ThresholdResult, ta
 
 	data := struct {
 		ResultStatus ResultStatus    `json:"result_status"`
-		RunStatus    lib.RunStatus   `json:"run_status"`
+		RunStatus    RunStatus       `json:"run_status"`
 		Thresholds   ThresholdResult `json:"thresholds"`
 	}{
 		resultStatus,
diff --git a/lib/run_status.go b/cloudapi/run_status.go
similarity index 71%
rename from lib/run_status.go
rename to cloudapi/run_status.go
index b331599ebf7..98500a63532 100644
--- a/lib/run_status.go
+++ b/cloudapi/run_status.go
@@ -1,10 +1,7 @@
-package lib
+package cloudapi
 
-// TODO: move to some other package - types? models?
-
-// RunStatus values can be used by k6 to denote how a script run ends
-// and by the cloud executor and collector so that k6 knows the current
-// status of a particular script run.
+// RunStatus values are used to tell the cloud output how a local test run
+// ended, and to get that information from the cloud for cloud tests.
 type RunStatus int
 
 // Possible run status values; iota isn't used intentionally
diff --git a/cmd/cloud.go b/cmd/cloud.go
index 1b43525c2c3..30104f9dd06 100644
--- a/cmd/cloud.go
+++ b/cmd/cloud.go
@@ -231,9 +231,9 @@ func (c *cmdCloud) run(cmd *cobra.Command, args []string) error {
 
 			statusText := testProgress.RunStatusText
 
-			if testProgress.RunStatus == lib.RunStatusFinished {
+			if testProgress.RunStatus == cloudapi.RunStatusFinished {
 				testProgress.Progress = 1
-			} else if testProgress.RunStatus == lib.RunStatusRunning {
+			} else if testProgress.RunStatus == cloudapi.RunStatusRunning {
 				if startTime.IsZero() {
 					startTime = time.Now()
 				}
@@ -270,8 +270,8 @@ func (c *cmdCloud) run(cmd *cobra.Command, args []string) error {
 		testProgress = newTestProgress
 		testProgressLock.Unlock()
 
-		if (newTestProgress.RunStatus > lib.RunStatusRunning) ||
-			(c.exitOnRunning && newTestProgress.RunStatus == lib.RunStatusRunning) {
+		if (newTestProgress.RunStatus > cloudapi.RunStatusRunning) ||
+			(c.exitOnRunning && newTestProgress.RunStatus == cloudapi.RunStatusRunning) {
 			globalCancel()
 			break
 		}
diff --git a/cmd/tests/cmd_test.go b/cmd/tests/cmd_test.go
index 8ceb7fa0112..7fcffd7e06f 100644
--- a/cmd/tests/cmd_test.go
+++ b/cmd/tests/cmd_test.go
@@ -24,7 +24,6 @@ import (
 	"go.k6.io/k6/cloudapi"
 	"go.k6.io/k6/cmd"
 	"go.k6.io/k6/errext/exitcodes"
-	"go.k6.io/k6/lib"
 	"go.k6.io/k6/lib/consts"
 	"go.k6.io/k6/lib/testutils"
 	"go.k6.io/k6/lib/testutils/httpmultibin"
@@ -424,7 +423,7 @@ func getTestServer(t *testing.T, routes map[string]http.Handler) *httptest.Serve
 	return httptest.NewServer(mux)
 }
 
-func getCloudTestEndChecker(t *testing.T, expRunStatus lib.RunStatus, expResultStatus cloudapi.ResultStatus) *httptest.Server {
+func getCloudTestEndChecker(t *testing.T, expRunStatus cloudapi.RunStatus, expResultStatus cloudapi.ResultStatus) *httptest.Server {
 	testFinished := false
 	srv := getTestServer(t, map[string]http.Handler{
 		"POST ^/v1/tests$": http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
@@ -445,7 +444,7 @@ func getCloudTestEndChecker(t *testing.T, expRunStatus lib.RunStatus, expResultS
 			runStatus := gjson.GetBytes(body, "run_status")
 			require.True(t, runStatus.Exists()) // important to check, since run_status can be 0
 			assert.Equalf(
-				t, expRunStatus, lib.RunStatus(runStatus.Int()),
+				t, expRunStatus, cloudapi.RunStatus(runStatus.Int()),
 				"received wrong run_status value",
 			)
 
@@ -469,7 +468,7 @@ func getCloudTestEndChecker(t *testing.T, expRunStatus lib.RunStatus, expResultS
 
 func getSimpleCloudOutputTestState(
 	t *testing.T, script string, cliFlags []string,
-	expRunStatus lib.RunStatus, expResultStatus cloudapi.ResultStatus, expExitCode exitcodes.ExitCode,
+	expRunStatus cloudapi.RunStatus, expResultStatus cloudapi.ResultStatus, expExitCode exitcodes.ExitCode,
 ) *GlobalTestState {
 	if cliFlags == nil {
 		cliFlags = []string{"-v", "--log-output=stdout"}
@@ -516,7 +515,7 @@ func TestSetupTeardownThresholds(t *testing.T) {
 		};
 	`)
 
-	ts := getSimpleCloudOutputTestState(t, script, nil, lib.RunStatusFinished, cloudapi.ResultStatusPassed, 0)
+	ts := getSimpleCloudOutputTestState(t, script, nil, cloudapi.RunStatusFinished, cloudapi.ResultStatusPassed, 0)
 	cmd.ExecuteWithGlobalState(ts.GlobalState)
 
 	stdOut := ts.Stdout.String()
@@ -562,7 +561,7 @@ func TestThresholdsFailed(t *testing.T) {
 	// Since these thresholds don't have an abortOnFail property, the run_status
 	// in the cloud will still be Finished, even if the test itself failed.
 	ts := getSimpleCloudOutputTestState(
-		t, script, nil, lib.RunStatusFinished, cloudapi.ResultStatusFailed, exitcodes.ThresholdsHaveFailed,
+		t, script, nil, cloudapi.RunStatusFinished, cloudapi.ResultStatusFailed, exitcodes.ThresholdsHaveFailed,
 	)
 	cmd.ExecuteWithGlobalState(ts.GlobalState)
 
@@ -603,7 +602,7 @@ func TestAbortedByThreshold(t *testing.T) {
 	`
 
 	ts := getSimpleCloudOutputTestState(
-		t, script, nil, lib.RunStatusAbortedThreshold, cloudapi.ResultStatusFailed, exitcodes.ThresholdsHaveFailed,
+		t, script, nil, cloudapi.RunStatusAbortedThreshold, cloudapi.ResultStatusFailed, exitcodes.ThresholdsHaveFailed,
 	)
 	cmd.ExecuteWithGlobalState(ts.GlobalState)
 
@@ -650,7 +649,7 @@ func TestAbortedByUserWithGoodThresholds(t *testing.T) {
 		};
 	`
 
-	ts := getSimpleCloudOutputTestState(t, script, nil, lib.RunStatusAbortedUser, cloudapi.ResultStatusPassed, exitcodes.ExternalAbort)
+	ts := getSimpleCloudOutputTestState(t, script, nil, cloudapi.RunStatusAbortedUser, cloudapi.ResultStatusPassed, exitcodes.ExternalAbort)
 
 	asyncWaitForStdoutAndStopTestWithInterruptSignal(t, ts, 15, time.Second, "simple iter 2")
 
@@ -778,7 +777,7 @@ func TestAbortedByUserWithRestAPI(t *testing.T) {
 
 	ts := getSimpleCloudOutputTestState(
 		t, script, []string{"-v", "--log-output=stdout", "--iterations", "20"},
-		lib.RunStatusAbortedUser, cloudapi.ResultStatusPassed, exitcodes.ScriptStoppedFromRESTAPI,
+		cloudapi.RunStatusAbortedUser, cloudapi.ResultStatusPassed, exitcodes.ScriptStoppedFromRESTAPI,
 	)
 
 	asyncWaitForStdoutAndStopTestFromRESTAPI(t, ts, 15, time.Second, "a simple iteration")
@@ -818,7 +817,7 @@ func TestAbortedByScriptSetupErrorWithDependency(t *testing.T) {
 		export { handleSummary } from "./bar.js";
 	`
 
-	srv := getCloudTestEndChecker(t, lib.RunStatusAbortedScriptError, cloudapi.ResultStatusPassed)
+	srv := getCloudTestEndChecker(t, cloudapi.RunStatusAbortedScriptError, cloudapi.ResultStatusPassed)
 
 	ts := NewGlobalTestState(t)
 	require.NoError(t, afero.WriteFile(ts.FS, filepath.Join(ts.Cwd, "test.js"), []byte(mainScript), 0o644))
@@ -933,7 +932,7 @@ func TestAbortedByScriptTeardownError(t *testing.T) {
 
 func testAbortedByScriptError(t *testing.T, script string, runTest func(*testing.T, *GlobalTestState)) *GlobalTestState {
 	ts := getSimpleCloudOutputTestState(
-		t, script, nil, lib.RunStatusAbortedScriptError, cloudapi.ResultStatusPassed, exitcodes.ScriptException,
+		t, script, nil, cloudapi.RunStatusAbortedScriptError, cloudapi.ResultStatusPassed, exitcodes.ScriptException,
 	)
 	runTest(t, ts)
 
@@ -1078,7 +1077,7 @@ func testAbortedByScriptTestAbort(
 	t *testing.T, shouldHaveMetrics bool, script string, runTest func(*testing.T, *GlobalTestState),
 ) *GlobalTestState { //nolint:unparam
 	ts := getSimpleCloudOutputTestState(
-		t, script, nil, lib.RunStatusAbortedUser, cloudapi.ResultStatusPassed, exitcodes.ScriptAborted,
+		t, script, nil, cloudapi.RunStatusAbortedUser, cloudapi.ResultStatusPassed, exitcodes.ScriptAborted,
 	)
 	runTest(t, ts)
 
@@ -1119,7 +1118,7 @@ func TestAbortedByInterruptDuringVUInit(t *testing.T) {
 	// This is testing the current behavior, which is expected, but it's not
 	// actually the desired one! See https://github.com/grafana/k6/issues/2804
 	ts := getSimpleCloudOutputTestState(
-		t, script, nil, lib.RunStatusAbortedSystem, cloudapi.ResultStatusPassed, exitcodes.GenericEngine,
+		t, script, nil, cloudapi.RunStatusAbortedSystem, cloudapi.ResultStatusPassed, exitcodes.GenericEngine,
 	)
 	asyncWaitForStdoutAndStopTestWithInterruptSignal(t, ts, 15, time.Second, "VU init sleeping for a while")
 	cmd.ExecuteWithGlobalState(ts.GlobalState)
@@ -1151,7 +1150,7 @@ func TestAbortedByScriptInitError(t *testing.T) {
 	`
 
 	ts := getSimpleCloudOutputTestState(
-		t, script, nil, lib.RunStatusAbortedScriptError, cloudapi.ResultStatusPassed, exitcodes.ScriptException,
+		t, script, nil, cloudapi.RunStatusAbortedScriptError, cloudapi.ResultStatusPassed, exitcodes.ScriptException,
 	)
 	cmd.ExecuteWithGlobalState(ts.GlobalState)
 
@@ -1249,7 +1248,7 @@ func TestMetricTagAndSetupDataIsolation(t *testing.T) {
 
 	ts := getSimpleCloudOutputTestState(
 		t, script, []string{"--quiet", "--log-output", "stdout"},
-		lib.RunStatusFinished, cloudapi.ResultStatusPassed, 0,
+		cloudapi.RunStatusFinished, cloudapi.ResultStatusPassed, 0,
 	)
 	cmd.ExecuteWithGlobalState(ts.GlobalState)
 
@@ -1419,7 +1418,7 @@ func TestMinIterationDuration(t *testing.T) {
 		export function teardown() { c.add(1); };
 	`
 
-	ts := getSimpleCloudOutputTestState(t, script, nil, lib.RunStatusFinished, cloudapi.ResultStatusPassed, 0)
+	ts := getSimpleCloudOutputTestState(t, script, nil, cloudapi.RunStatusFinished, cloudapi.ResultStatusPassed, 0)
 
 	start := time.Now()
 	cmd.ExecuteWithGlobalState(ts.GlobalState)
diff --git a/core/engine.go b/core/engine.go
index ed98d744cbf..7afcf1660cf 100644
--- a/core/engine.go
+++ b/core/engine.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/sirupsen/logrus"
 
+	"go.k6.io/k6/cloudapi"
 	"go.k6.io/k6/errext"
 	"go.k6.io/k6/errext/exitcodes"
 	"go.k6.io/k6/lib"
@@ -151,11 +152,11 @@ func (e *Engine) setRunStatusFromError(err error) {
 	var serr errext.Exception
 	switch {
 	case errors.As(err, &serr):
-		e.OutputManager.SetRunStatus(lib.RunStatusAbortedScriptError)
+		e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedScriptError)
 	case errext.IsInterruptError(err):
-		e.OutputManager.SetRunStatus(lib.RunStatusAbortedUser)
+		e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedUser)
 	default:
-		e.OutputManager.SetRunStatus(lib.RunStatusAbortedSystem)
+		e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedSystem)
 	}
 }
 
@@ -198,23 +199,23 @@ func (e *Engine) startBackgroundProcesses(
 				e.setRunStatusFromError(err)
 			} else {
 				e.logger.Debug("run: execution scheduler finished nominally")
-				e.OutputManager.SetRunStatus(lib.RunStatusFinished)
+				e.OutputManager.SetRunStatus(cloudapi.RunStatusFinished)
 			}
 			// do nothing, return the same err value we got from the Run()
 			// ExecutionScheduler result, we just set the run_status based on it
 		case <-runCtx.Done():
 			e.logger.Debug("run: context expired; exiting...")
-			e.OutputManager.SetRunStatus(lib.RunStatusAbortedUser)
+			e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedUser)
 			err = errext.WithExitCodeIfNone(errors.New("test run aborted by signal"), exitcodes.ExternalAbort)
 		case <-e.stopChan:
 			runSubCancel()
 			e.logger.Debug("run: stopped by user via REST API; exiting...")
-			e.OutputManager.SetRunStatus(lib.RunStatusAbortedUser)
+			e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedUser)
 			err = errext.WithExitCodeIfNone(errors.New("test run stopped from the REST API"), exitcodes.ScriptStoppedFromRESTAPI)
 		case <-thresholdAbortChan:
 			e.logger.Debug("run: stopped by thresholds; exiting...")
 			runSubCancel()
-			e.OutputManager.SetRunStatus(lib.RunStatusAbortedThreshold)
+			e.OutputManager.SetRunStatus(cloudapi.RunStatusAbortedThreshold)
 			err = errext.WithExitCodeIfNone(errors.New("test run aborted by failed thresholds"), exitcodes.ThresholdsHaveFailed)
 		}
 	}()
diff --git a/lib/testutils/mockoutput/mockoutput.go b/lib/testutils/mockoutput/mockoutput.go
index f99b128d080..6d73123eb63 100644
--- a/lib/testutils/mockoutput/mockoutput.go
+++ b/lib/testutils/mockoutput/mockoutput.go
@@ -1,9 +1,7 @@
 package mockoutput
 
 import (
-	"go.k6.io/k6/lib"
 	"go.k6.io/k6/metrics"
-	"go.k6.io/k6/output"
 )
 
 // New exists so that the usage from tests avoids repetition, i.e. is
@@ -16,15 +14,12 @@ func New() *MockOutput {
 type MockOutput struct {
 	SampleContainers []metrics.SampleContainer
 	Samples          []metrics.Sample
-	RunStatus        lib.RunStatus
 
 	DescFn  func() string
 	StartFn func() error
 	StopFn  func() error
 }
 
-var _ output.WithRunStatusUpdates = &MockOutput{}
-
 // AddMetricSamples just saves the results in memory.
 func (mo *MockOutput) AddMetricSamples(scs []metrics.SampleContainer) {
 	mo.SampleContainers = append(mo.SampleContainers, scs...)
@@ -33,11 +28,6 @@ func (mo *MockOutput) AddMetricSamples(scs []metrics.SampleContainer) {
 	}
 }
 
-// SetRunStatus updates the RunStatus property.
-func (mo *MockOutput) SetRunStatus(latestStatus lib.RunStatus) {
-	mo.RunStatus = latestStatus
-}
-
 // Description calls the supplied DescFn callback, if available.
 func (mo *MockOutput) Description() string {
 	if mo.DescFn != nil {
diff --git a/output/cloud/output.go b/output/cloud/output.go
index ee98b193b4d..0a73c5f16e5 100644
--- a/output/cloud/output.go
+++ b/output/cloud/output.go
@@ -36,7 +36,7 @@ type Output struct {
 	thresholds    map[string][]*metrics.Threshold
 	client        *MetricsClient
 
-	runStatus lib.RunStatus
+	runStatus cloudapi.RunStatus
 
 	bufferMutex      sync.Mutex
 	bufferHTTPTrails []*httpext.Trail
@@ -284,7 +284,7 @@ func (out *Output) Description() string {
 }
 
 // SetRunStatus receives the latest run status from the Engine.
-func (out *Output) SetRunStatus(status lib.RunStatus) {
+func (out *Output) SetRunStatus(status cloudapi.RunStatus) {
 	out.runStatus = status
 }
 
@@ -634,8 +634,8 @@ func (out *Output) testFinished() error {
 		}
 	}
 
-	runStatus := lib.RunStatusFinished
-	if out.runStatus != lib.RunStatusQueued {
+	runStatus := cloudapi.RunStatusFinished
+	if out.runStatus != cloudapi.RunStatusQueued {
 		runStatus = out.runStatus
 	}
 
diff --git a/output/cloud/output_test.go b/output/cloud/output_test.go
index 86a0778b181..6cd04612219 100644
--- a/output/cloud/output_test.go
+++ b/output/cloud/output_test.go
@@ -506,7 +506,7 @@ func testCloudOutputStopSendingMetric(t *testing.T, stopOnError bool) {
 
 	require.NoError(t, out.Stop())
 
-	require.Equal(t, lib.RunStatusQueued, out.runStatus)
+	require.Equal(t, cloudapi.RunStatusQueued, out.runStatus)
 	select {
 	case <-out.stopSendingMetrics:
 		// all is fine
@@ -598,7 +598,7 @@ func TestCloudOutputAggregationPeriodZeroNoBlock(t *testing.T) {
 	tb.Mux.HandleFunc(fmt.Sprintf("/v1/metrics/%s", out.referenceID), getSampleChecker(t, expSamples))
 
 	require.NoError(t, out.Stop())
-	require.Equal(t, lib.RunStatusQueued, out.runStatus)
+	require.Equal(t, cloudapi.RunStatusQueued, out.runStatus)
 }
 
 func TestCloudOutputPushRefID(t *testing.T) {
diff --git a/output/manager.go b/output/manager.go
index 38d72e53b78..1ab3ba29ca5 100644
--- a/output/manager.go
+++ b/output/manager.go
@@ -2,7 +2,7 @@ package output
 
 import (
 	"github.com/sirupsen/logrus"
-	"go.k6.io/k6/lib"
+	"go.k6.io/k6/cloudapi"
 	"go.k6.io/k6/metrics"
 )
 
@@ -58,7 +58,7 @@ func (om *Manager) stopOutputs(upToID int) {
 
 // SetRunStatus checks which outputs implement the WithRunStatusUpdates
 // interface and sets the provided RunStatus to them.
-func (om *Manager) SetRunStatus(status lib.RunStatus) {
+func (om *Manager) SetRunStatus(status cloudapi.RunStatus) {
 	for _, out := range om.outputs {
 		if statUpdOut, ok := out.(WithRunStatusUpdates); ok {
 			statUpdOut.SetRunStatus(status)
diff --git a/output/types.go b/output/types.go
index 09885d03566..92a2648d1d0 100644
--- a/output/types.go
+++ b/output/types.go
@@ -11,6 +11,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/afero"
 
+	"go.k6.io/k6/cloudapi"
 	"go.k6.io/k6/lib"
 	"go.k6.io/k6/metrics"
 )
@@ -78,7 +79,7 @@ type WithTestRunStop interface {
 // WithRunStatusUpdates means the output can receive test run status updates.
 type WithRunStatusUpdates interface {
 	Output
-	SetRunStatus(latestStatus lib.RunStatus)
+	SetRunStatus(latestStatus cloudapi.RunStatus)
 }
 
 // WithBuiltinMetrics means the output can receive the builtin metrics.