Conversation
| require.NoError(t, err) | ||
|
|
||
| // test to ensure all three components - core.loadavg, docker.logs, and docker.daemon - became specs | ||
| loadSpec, logsSpec, daemonSpec := false, false, false |
There was a problem hiding this comment.
I think require.Contains would be tidier
| ) | ||
|
|
||
| // Test that yml files are parsed into spec lists properly | ||
| func TestParse(t *testing.T) { |
There was a problem hiding this comment.
This could be unit test for Parse in the spec package, just remove the planner test at the end.
There was a problem hiding this comment.
Sounds like a good idea
|
|
||
| // TestGenerate runs all the local data collection tools (read file, run command, hostname, loadavg, uptime) | ||
| // some tasks are not fully tested on windows (run command, loadavg, uptime) | ||
| func TestGenerate(t *testing.T) { |
There was a problem hiding this comment.
I'm a bit weary of maintaining this test because it's written like a unit test but the scope is of an integration test.
I think it would be better to break this down into several smaller tests. You could have one unit test for the Generate function with a set of fake Tasks with prepared results, and one integration test for each planner.
For the planner tests, I'd wait until we have the CLI working and write them as blackbox tests with Ginkgo. Each test would be a yaml spec and assert that the returned archive contains the expected files.
There was a problem hiding this comment.
I agree - it really does have the scope of an integration test. I'll write a test for the Generate function within the relevant package, but leave this here for the moment to reuse when we make ginkgo integration tests, if that makes sense.
areed
left a comment
areed
left a comment
There was a problem hiding this comment.
Couple small style items. The generate test turned out nice.
| "github.com/replicatedcom/support-bundle/types" | ||
| ) | ||
|
|
||
| // TestGenerate runs all the local data collection tools (read file, run command, hostname, loadavg, uptime) |
| tasks := []types.Task{singleResults, mixedResults} | ||
|
|
||
| got, _ := ioutil.TempFile("", "generate-test-bundle") | ||
| // fmt.Println(got.Name()) |
| err := Generate(tasks, time.Duration(time.Second*2), got.Name()) | ||
| require.NoError(t, err) | ||
|
|
||
| testDir, _ := ioutil.TempDir("", "generate-test") |
There was a problem hiding this comment.
Any reason not to require.NoError(t, err) here?
|
|
||
| // Test that yml files are parsed into spec lists properly | ||
| func TestParse(t *testing.T) { | ||
| yml, err := ioutil.ReadFile("./spec.yml") |
There was a problem hiding this comment.
Could you inline this file since this is a unit test? It makes the test easier to read and doesn't pollute the package directory.
| logsSpec = true | ||
| require.Equal(t, "/raw/containers/testExample/logs.txt", spec.Raw) | ||
| require.NotNil(t, spec.Config) | ||
| // configMap, ok := spec.Config.(map[string]string) |
| Human: "/human/metrics/loadavg", | ||
| }) | ||
|
|
||
| // require.Contains(t, specs, types.Spec{ |
| specs, err := Parse(yml) | ||
| require.NoError(t, err) | ||
|
|
||
| // test to ensure basics of docker.logs, since |
There was a problem hiding this comment.
It would be nice to use the require.Contains format for this test too. Did you try something like
var config = map[interface{}]interface{}
config["key"] = "val"
types.Spec{
Builtin: "docker.logs",
Config: config,
}
There was a problem hiding this comment.
I had tried with map[string]string, which didn't work - I'll try with map[interface{}]interface{}
There was a problem hiding this comment.
https://github.com/replicatedcom/support-bundle/blob/master/plugins/docker/planners/logs.go#L10
Here's the code that parses that config. Maybe you learned a cleaner way to do this when working on Replicated yaml?
| // these commands aren't tested on windows platforms | ||
| lsCommandPath := "" | ||
| sleepCommandPath := "" | ||
| // sleepCommandPath := "" |
| require.NotEqual(t, "", resultInfo.Path) | ||
| lsCommandPath = resultInfo.Path | ||
| } | ||
| // if resultInfo.Task == "runCommand" && resultInfo.Args[0] == "sleep" && resultInfo.Args[1] == "1m" { |
laverya
force-pushed
the
integration-test-fixes
branch
from
156d52e to
f6bcc5e
Compare
Reformatted integration test to use new task format read-command.go modified to use an ellipsis operator cmd.generate.go updated to use new function signatures Added small test of docker planner to the integration test Fix uptime and loadavg tests Created parser test Created much more limited test for bundle.Generate
laverya
force-pushed
the
integration-test-fixes
branch
from
f6bcc5e to
c824b76
Compare
2 participants
The timeout-dependent tests don't work (since there aren't errors generated, and we can't set per-task timeouts) but the rest does.