Create distinct test state objects for the pre-init and run phases and thread them everywhere

[na--]

While all of the commits here should be self-contained, a lot of them change the same lines of code multiple times, so it might be easier to just review the final diff.

This refactoring was prompted by desire in #2594 (comment) to be able to construct some object out of the final (i.e. consolidated, validated and derived) script Options and make it accessible to the various k6 components (e.g. executors, JS APIs, etc.) while the test is running. It turns out that it was basically impossible, and previous things that needed to be threaded through the codebase like that were done in an ad-hoc manner, mostly by adding extra function arguments wherever they were needed 😱

The only exception to that was the lib.RuntimeState, which was added to #2487. Unfortunately, it only could contain things that were available before the initial init context execution and the creation of the (badly named) Runner 😞 Moreover, on a second look, the name RuntimeState was a bit misleading to me, since it clashes a bit with the goja JS "runtime" we have.

So, the TLDR version is that I decided go down this rabbit hole and rename RuntimeState to TestPreInitState and also introduce a new TestRunState struct that builds on top of, and to thread it everywhere I could 😅 We went from this:

k6/lib/runtime_state.go

Lines 10 to 18 in 1e84682

	// RuntimeState represents what is mostly needed during the running of a test
	type RuntimeState struct {
	RuntimeOptions RuntimeOptions
	// TODO maybe have a struct `Metrics` with `Registry` and `Builtin` ?
	Registry *metrics.Registry
	BuiltinMetrics *metrics.BuiltinMetrics
	KeyLogger io.Writer
	Logger *logrus.Logger
	}

To this:

k6/lib/test_state.go

Lines 10 to 34 in 69a0b77

	// TestPreInitState contains all of the state that can be gathered and built
	// before the test run is initialized.
	type TestPreInitState struct {
	RuntimeOptions RuntimeOptions
	Registry *metrics.Registry
	BuiltinMetrics *metrics.BuiltinMetrics
	KeyLogger io.WriteCloser
	// TODO: replace with logrus.FieldLogger when all of the tests can be fixed
	Logger *logrus.Logger
	}
	// TestRunState contains the pre-init state as well as all of the state and
	// options that are necessary for actually running the test.
	type TestRunState struct {
	*TestPreInitState
	Options Options
	Runner Runner // TODO: rename to something better, see type comment
	// TODO: add atlas root node
	// TODO: add other properties that are computed or derived after init, e.g.
	// thresholds?
	}

And from this:

k6/cmd/run.go

Line 92 in 1e84682

execScheduler, err := local.NewExecutionScheduler(test.initRunner, test.builtInMetrics, logger)

k6/cmd/run.go

Lines 128 to 131 in 1e84682

	engine, err := core.NewEngine(
	execScheduler, conf.Options, test.runtimeOptions,
	outputs, logger, test.metricsRegistry,
	)

k6/core/engine.go

Line 92 in 1e84682

me, err := engine.NewMetricsEngine(registry, ex.GetState(), opts, rtOpts, logger)

To this:

k6/cmd/run.go

Line 93 in 69a0b77

execScheduler, err := local.NewExecutionScheduler(testRunState)

k6/cmd/run.go

Line 129 in 69a0b77

engine, err := core.NewEngine(testRunState, execScheduler, outputs)

k6/core/engine.go

Line 89 in 69a0b77

me, err := engine.NewMetricsEngine(ex.GetState())

Besides the simplified invocations, the biggest benefit is that we can now easily add things to the TestPreInitState and TestRunState structs and have them available everywhere 🎉 Most of the rest of the changes here were just me fixing the endless tests that broke from the change APIs 😭

[codebien]

Suggested change

	type TestRunState struct {
	type TestRun struct {

Do we really need the word State? Every struct is a state of something in the end.

[na--]

My logic is that TestPreInitState contains the state needed by the test before it's initialized ("pre-init state"), and TestRunState contains the state needed by the test while it's running ("run state")

[na--]

hmm though the Runner and Options are not exactly "state"... 😅 🤷‍♂️

👍 from me for renaming it to just TestRun - I am fine with either name, what do the rest of the @grafana/k6-devs think?

[oleiade]

Personally, I find TestRunState more explicit. It's somewhat consistent with the rest of our naming, where we already have things like ExecutionState and RuntimeOptions, with a clear distinction between State (something is running, and this is where its knobs are at), and Options (whenever you need to decide what behavior to observe, or which state to go in, look at those values).

But I agree that we do have a mix of state and config in there at the moment.

[mstoykov]

LGTM in general 👏 !

I have left some nitpick comments but nothing blocking.

I do like how some of the signatures are now smaller and we have pre and post init state structs.

But I am a bit worried that now that we provide a whole struct with k fields and not just the 2 things that will be used from it. This might lead to strange and hard to diagnose issues where in some test or the prod code we forgot to set a field. Hopefully this will be caught by tests when the change is made.

[oleiade]

Besides a question about naming/documentation, LGTM 🚀 🦕