flowtest: introduce script-based testing framework for Flow #6401

rfratto · 2024-02-20T19:03:25Z

This commit introduces a new package, flow/flowtest, which can be used for running script-based test using Flow components.

flowtest accepts a txtar archive, where the comment of the txtar archive denotes the script to run. Files in the txtar archive are unpacked to a temporary directory as part of the test; that temporary directory is also used as the working directory of the Flow controller.

See https://rsc.io/script for more information about the scripting language and available default commands.

The true value of this package is unknown, but it is expected to be useful in some cases as an alternative to writing pure Go tests. It is likely that more commands will be needed over time to cover more use cases, and we may find ourselves replacing existing tests with script-based tests where appropriate.

Flow-specific script commands

In addition to the default commands (cd, cat, exec, etc.), the following Flow-specific commands are made available to scripts:

controller.load [file]: Load a file into the Flow controller.
controller.start: Start the Flow controller.
assert.health [component] [health]: Assert the health of a specific component.

Note that controller.start should almost always be run as a background command to allow the controller to run for the duration of the test.

Users of flowtest can provide custom commands on top of these by passing WithExtraCommands to TestScript.

Example test script

The following is an example script using these commands:

# This file performs a basic test of loading a component and asserting its
# health.

controller.load main.river
controller.start &

assert.health local.file.example healthy

-- main.river --
local.file "example" {
  filename = "./hello.txt"
}

-- hello.txt --
Hello, world!

See the testdata/ directory for other test examples.

This commit introduces a new package, flow/flowtest, which can be used for running script-based test using Flow components. flowtest accepts a txtar archive, where the comment of the txtar archive denotes the script to run. Files in the txtar archive are unpacked to a temporary directory as part of the test; that temporary directory is also used as the working directory of the Flow controller. See <rsc.io/script> for more information about the scripting language and available default commands. In addition to the default commands, the following Flow-specific commands are made available to scripts: - `controller.load [file]`: Load a file into the Flow controller. - `controller.start`: Start the Flow controller. - `assert.health [component] [health]`: Assert the health of a specific component. Note that `controller.start` should almost always be run as a background command to allow the controller to run for the duration of the test. Users of flowtest can provide custom commands on top of these by passing `WithExtraCommands` to `TestScript`. The following is an example script using these commands: # This file performs a basic test of loading a component and asserting its # health. controller.load main.river controller.start & assert.health local.file.example healthy -- main.river -- local.file "example" { filename = "./hello.txt" } -- hello.txt -- Hello, world! See the testdata/ directory for other test examples.

thampiotr · 2024-02-21T10:29:27Z

pkg/flow/flowtest/commands.go

+			Summary: "start the Flow controller",
+			Async:   true,
+			Detail: []string{
+				"This command should almost always be run in the background by appending a &, otherwise the controller will exit immediately after the command finishes'",


if & is missing, wouldn't the script block execution until controller exits (which could be forever)?

thampiotr · 2024-02-21T10:30:46Z

pkg/flow/flowtest/commands.go

+	return cmds
+}
+
+func registerControllerCommands(f *flow.Flow, cmds map[string]script.Cmd) {


It's quite a long function, maybe we should put each command in its own function.

thampiotr · 2024-02-21T10:37:36Z

pkg/flow/flowtest/commands.go

+		},
+	)
+
+	cmds["controller.start"] = script.Command(


I thought it would make most sense to be as close to user experience as possible with those tests so things like quirks of CLI parsing or conflicting flag combinations or environment variables can be tested. Also, we can completely refactor things inside and validate that the users' interface remains the same.

The users call grafana-agent run <config file> and they don't start a controller themselves, so I would think that instead of operating on the controller level we should operate on agent's CLI level? We'd still need a way to extract controller from a running agent for the purposes of asserting on conditions like 'healthy', which is fine.

thampiotr · 2024-02-21T10:38:55Z

pkg/flow/flowtest/commands.go

+}
+
+func registerControllerCommands(f *flow.Flow, cmds map[string]script.Cmd) {
+	cmds["controller.load"] = script.Command(


How would testing of two agents work? for example a proxy scenario or a cluster of agents?

thampiotr · 2024-02-21T10:40:00Z

pkg/flow/flowtest/testdata/basic.txtar

+
+assert.health local.file.example healthy
+
+-- main.river --


I do like the embedded files :)

thampiotr · 2024-02-21T10:42:30Z

pkg/flow/flowtest/flowtest.go

+// In addition to the default commands provided by [rsc.io/script], the
+// following Flow-specific commands are available:
+//
+//   - `controller.load [file]`: Load a file into the Flow controller.
+//   - `controller.start`: Start the Flow controller.
+//   - `assert.health [component] [health]`: Assert the health of a specific component.


I think we should just point to the file that registers all these commands. This is likely to get outdated fast.

thampiotr · 2024-02-21T10:47:22Z

pkg/flow/flowtest/flowtest_test.go

+			return nil
+		}
+		t.Run(d.Name(), func(t *testing.T) {
+			require.NoError(t, flowtest.TestScript(path))


Could we instead pass the testing.T to TestScript and further, so that we can use the rich assert/require library? These can facilitate stuff like comparing maps, checking numbers are within a range, checking something is contained in the other thing, using the 'eventually' functions... the error messages produced by these are usually quite clear and useful. So I think it's worth to make it possible to use them - especially will be useful for all the asserting commands.

thampiotr · 2024-02-21T10:51:22Z

pkg/flow/flowtest/flowtest.go

+
+	// Create a test controller for the script to interact with. The test
+	// controller's logs are buffered and printed to stderr on exit. This
+	// prevents log mangling when both the script engine and the Flow controller


We could put prefixes in logs and print at the same time? Splitting them becomes a problem when we need to correlate the logs of script with the logs of the controller to see what happened in what order.

ptodev

Thank you, this looks interesting and I look forward to using it! I wonder how convenient it will be with debuggers too. E.g. I guess there won't be any issues with the temp folders if I terminate the process mid-session while debugging.

ptodev · 2024-02-22T17:06:36Z

go.mod

@@ -583,7 +583,7 @@ require (
 	golang.org/x/mod v0.14.0 // indirect
 	golang.org/x/sync v0.5.0 // indirect
 	golang.org/x/term v0.16.0 // indirect
-	golang.org/x/tools v0.15.0 // indirect
+	golang.org/x/tools v0.15.0


It'd be nice if this this goes in the first import block with direct dependencies

ptodev · 2024-02-22T17:23:05Z

pkg/flow/flowtest/flowtest.go

+	}
+
+	// Create a temporary directory for the scope of our test to operate in.
+	tmpDir, err := os.MkdirTemp("", "flowtest-*")


Are those directories cleaned up?

github-actions · 2024-03-24T00:11:43Z

This PR has not had any activity in the past 30 days, so the needs-attention label has been added to it.
If you do not have enough time to follow up on this PR or you think it's no longer relevant, consider closing it.
The needs-attention label signals to maintainers that something has fallen through the cracks. No action is needed by you; your PR will be kept open and you do not have to respond to this comment. The label will be removed the next time this job runs if there is new activity.
Thank you for your contributions!

github-actions · 2024-05-14T00:10:46Z

This PR has not had any activity in the past 30 days, so the needs-attention label has been added to it.
If you do not have enough time to follow up on this PR or you think it's no longer relevant, consider closing it.
The needs-attention label signals to maintainers that something has fallen through the cracks. No action is needed by you; your PR will be kept open and you do not have to respond to this comment. The label will be removed the next time this job runs if there is new activity.
Thank you for your contributions!

rfratto · 2024-05-14T00:14:05Z

I'm going to close this for now. I'm still interested in a script-based testing framework, but it sounds like we need more thoughts around what the scripts should look like (and how to use them to test things like clustering).

@thampiotr expressed interest in taking inspiration from this exploration, so I'm handing it over to him for future work on this :)

thampiotr reviewed Feb 21, 2024

View reviewed changes

ptodev reviewed Feb 22, 2024

View reviewed changes

github-actions bot added the needs-attention An issue or PR has been sitting around and needs attention. label Mar 24, 2024

rfratto added variant/flow Relatd to Grafana Agent Flow. enhancement New feature or request labels Apr 9, 2024

github-actions bot removed the needs-attention An issue or PR has been sitting around and needs attention. label Apr 11, 2024

github-actions bot added the needs-attention An issue or PR has been sitting around and needs attention. label May 14, 2024

rfratto closed this May 14, 2024

github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Jun 14, 2024

github-actions bot locked as resolved and limited conversation to collaborators Jun 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

flowtest: introduce script-based testing framework for Flow #6401

flowtest: introduce script-based testing framework for Flow #6401

rfratto commented Feb 20, 2024

thampiotr Feb 21, 2024

thampiotr Feb 21, 2024

thampiotr Feb 21, 2024

thampiotr Feb 21, 2024

thampiotr Feb 21, 2024

thampiotr Feb 21, 2024

thampiotr Feb 21, 2024

thampiotr Feb 21, 2024

ptodev left a comment

ptodev Feb 22, 2024

ptodev Feb 22, 2024

github-actions bot commented Mar 24, 2024

github-actions bot commented May 14, 2024

rfratto commented May 14, 2024

flowtest: introduce script-based testing framework for Flow #6401

flowtest: introduce script-based testing framework for Flow #6401

Conversation

rfratto commented Feb 20, 2024

Flow-specific script commands

Example test script

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ptodev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 24, 2024

github-actions bot commented May 14, 2024

rfratto commented May 14, 2024