-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tool test: improving coverage, cleanups, minor fixes #474
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,9 @@ | |
"github.com/bazelbuild/remote-apis-sdks/go/pkg/fakes" | ||
"github.com/bazelbuild/remote-apis-sdks/go/pkg/outerr" | ||
"github.com/google/go-cmp/cmp" | ||
"google.golang.org/protobuf/encoding/prototext" | ||
|
||
repb "github.com/bazelbuild/remote-apis/build/bazel/remote/execution/v2" | ||
) | ||
|
||
func TestTool_DownloadActionResult(t *testing.T) { | ||
|
@@ -83,8 +86,8 @@ | |
|
||
Inputs | ||
====== | ||
[Root directory digest: e23e10be0d14b5b2b1b7af32de78dea554a74df5bb22b31ae6c49583c1a8aa0e/75] | ||
a/b/input.txt: [File digest: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855/0] | ||
[Root directory digest: 1b1d5b9e3eb97866805deca901299fcdde98b5747e147f35187e12c16e120186/75] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's from #473 |
||
a/b/input.txt: [File digest: c96c6d5be8d08a12e7b5cdc1b207fa6b2430974c86803d8891675e76fd992c20/5] | ||
|
||
------------------------------------------------------------------------ | ||
Action Result | ||
|
@@ -109,55 +112,98 @@ | |
e, cleanup := fakes.NewTestEnv(t) | ||
defer cleanup() | ||
cmd := &command.Command{ | ||
Args: []string{"foo bar baz"}, | ||
Args: []string{"foo", "bar", "baz"}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is here (and below) in order to not provide bad examples in our unit tests. E.g. "echo hello world" would not work as a single string; it needs to be split up. |
||
ExecRoot: e.ExecRoot, | ||
InputSpec: &command.InputSpec{Inputs: []string{"i1", "i2"}}, | ||
OutputFiles: []string{"a/b/out"}, | ||
} | ||
if err := os.WriteFile(filepath.Join(e.ExecRoot, "i1"), []byte("i1"), 0644); err != nil { | ||
t.Fatalf("failed creating input file: %v", err) | ||
} | ||
if err := os.WriteFile(filepath.Join(e.ExecRoot, "i2"), []byte("i2"), 0644); err != nil { | ||
t.Fatalf("failed creating input file: %v", err) | ||
} | ||
out := "output" | ||
opt := &command.ExecutionOptions{AcceptCached: false, DownloadOutputs: true, DownloadOutErr: true} | ||
_, acDg, _, _ := e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: out}) | ||
_, acDg, _, _ := e.Set(cmd, command.DefaultExecutionOptions(), &command.Result{Status: command.SuccessResultStatus}, &fakes.InputFile{Path: "i1", Contents: "i1"}, &fakes.InputFile{Path: "i2", Contents: "i2"}, &fakes.OutputFile{Path: "a/b/out", Contents: "out"}) | ||
|
||
client := &Client{GrpcClient: e.Client.GrpcClient} | ||
if err := client.CheckDeterminism(context.Background(), acDg.String(), "", 2); err != nil { | ||
t.Errorf("CheckDeterminism returned an error: %v", err) | ||
} | ||
// Now execute again with changed inputs. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment was a result of copy-paste; inputs do not actually change, only output does. |
||
// Now execute again and return a different output. | ||
testOnlyStartDeterminismExec = func() { | ||
out = "output2" | ||
e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: out}) | ||
e.Set(cmd, command.DefaultExecutionOptions(), &command.Result{Status: command.SuccessResultStatus}, &fakes.InputFile{Path: "i1", Contents: "i1"}, &fakes.InputFile{Path: "i2", Contents: "i2"}, &fakes.OutputFile{Path: "a/b/out", Contents: "out2"}) | ||
} | ||
defer func() { testOnlyStartDeterminismExec = func() {} }() | ||
if err := client.CheckDeterminism(context.Background(), acDg.String(), "", 2); err == nil { | ||
t.Errorf("CheckDeterminism returned nil, want error") | ||
} | ||
} | ||
|
||
func TestTool_ExecuteAction(t *testing.T) { | ||
func TestTool_DownloadAction(t *testing.T) { | ||
e, cleanup := fakes.NewTestEnv(t) | ||
defer cleanup() | ||
cmd := &command.Command{ | ||
Args: []string{"foo bar baz"}, | ||
Args: []string{"foo", "bar", "baz"}, | ||
ExecRoot: e.ExecRoot, | ||
InputSpec: &command.InputSpec{Inputs: []string{"i1", "i2"}}, | ||
InputSpec: &command.InputSpec{Inputs: []string{"i1", "a/b/i2"}}, | ||
OutputFiles: []string{"a/b/out"}, | ||
} | ||
if err := os.WriteFile(filepath.Join(e.ExecRoot, "i1"), []byte("i1"), 0644); err != nil { | ||
t.Fatalf("failed creating input file: %v", err) | ||
_, acDg, _, _ := e.Set(cmd, command.DefaultExecutionOptions(), &command.Result{Status: command.SuccessResultStatus}, &fakes.InputFile{Path: "i1", Contents: "i1"}, &fakes.InputFile{Path: "a/b/i2", Contents: "i2"}) | ||
|
||
client := &Client{GrpcClient: e.Client.GrpcClient} | ||
tmpDir := filepath.Join(t.TempDir(), "action_root") | ||
os.MkdirAll(tmpDir, os.ModePerm) | ||
if err := client.DownloadAction(context.Background(), acDg.String(), tmpDir); err != nil { | ||
t.Errorf("error DownloadAction: %v", err) | ||
} | ||
if err := os.WriteFile(filepath.Join(e.ExecRoot, "i2"), []byte("i2"), 0644); err != nil { | ||
t.Fatalf("failed creating input file: %v", err) | ||
|
||
reCmdPb := &repb.Command{ | ||
Arguments: []string{"foo", "bar", "baz"}, | ||
OutputFiles: []string{"a/b/out"}, | ||
} | ||
acPb := &repb.Action{ | ||
CommandDigest: digest.TestNewFromMessage(reCmdPb).ToProto(), | ||
InputRootDigest: &repb.Digest{Hash: "2f33e7247d11728cfc1515b378040ecf3bd8be4eb4330ca7b6023b23c43b348b", SizeBytes: 153}, | ||
} | ||
expectedContents := []struct { | ||
path string | ||
contents string | ||
}{ | ||
{ | ||
path: "ac.textproto", | ||
contents: prototext.Format(acPb), | ||
}, | ||
{ | ||
path: "cmd.textproto", | ||
contents: prototext.Format(reCmdPb), | ||
}, | ||
{ | ||
path: "input/i1", | ||
contents: "i1", | ||
}, | ||
{ | ||
path: "input/a/b/i2", | ||
contents: "i2", | ||
}, | ||
} | ||
for _, ec := range expectedContents { | ||
fp := filepath.Join(tmpDir, ec.path) | ||
got, err := os.ReadFile(fp) | ||
if err != nil { | ||
t.Fatalf("Unable to read downloaded action file %v: %v", fp, err) | ||
} | ||
if diff := cmp.Diff(ec.contents, string(got)); diff != "" { | ||
t.Errorf("Incorrect content in downloaded file %v: diff (-want +got): %v\n\ngot: %s\n\nwant: %v\n", fp, diff, got, ec.contents) | ||
} | ||
} | ||
} | ||
|
||
func TestTool_ExecuteAction(t *testing.T) { | ||
e, cleanup := fakes.NewTestEnv(t) | ||
defer cleanup() | ||
cmd := &command.Command{ | ||
Args: []string{"foo", "bar", "baz"}, | ||
ExecRoot: e.ExecRoot, | ||
InputSpec: &command.InputSpec{Inputs: []string{"i1", "i2"}}, | ||
OutputFiles: []string{"a/b/out"}, | ||
} | ||
out := "output" | ||
opt := &command.ExecutionOptions{AcceptCached: false, DownloadOutputs: true, DownloadOutErr: true} | ||
_, acDg, _, _ := e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: out}, | ||
fakes.StdOut("stdout"), fakes.StdErr("stderr")) | ||
_, acDg, _, _ := e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: "out"}, | ||
&fakes.InputFile{Path: "i1", Contents: "i1"}, &fakes.InputFile{Path: "i2", Contents: "i2"}, fakes.StdOut("stdout"), fakes.StdErr("stderr")) | ||
|
||
client := &Client{GrpcClient: e.Client.GrpcClient} | ||
oe := outerr.NewRecordingOutErr() | ||
|
@@ -171,16 +217,24 @@ | |
t.Errorf("Incorrect stdout %v, expected \"stdout\"", oe.Stdout()) | ||
} | ||
// Now execute again with changed inputs. | ||
tmpDir := t.TempDir() | ||
if err := os.WriteFile(filepath.Join(tmpDir, "i1"), []byte("i11"), 0644); err != nil { | ||
t.Fatalf("failed creating input file: %v", err) | ||
tmpDir := filepath.Join(t.TempDir(), "action_root") | ||
os.MkdirAll(tmpDir, os.ModePerm) | ||
inputRoot := filepath.Join(tmpDir, "input") | ||
if err := client.DownloadAction(context.Background(), acDg.String(), tmpDir); err != nil { | ||
t.Errorf("error DownloadAction: %v", err) | ||
} | ||
if err := os.WriteFile(filepath.Join(tmpDir, "i2"), []byte("i22"), 0644); err != nil { | ||
t.Fatalf("failed creating input file: %v", err) | ||
if err := os.WriteFile(filepath.Join(inputRoot, "i1"), []byte("i11"), 0644); err != nil { | ||
t.Fatalf("failed overriding input file: %v", err) | ||
} | ||
if err := os.WriteFile(filepath.Join(inputRoot, "i2"), []byte("i22"), 0644); err != nil { | ||
t.Fatalf("failed overriding input file: %v", err) | ||
} | ||
cmd.ExecRoot = tmpDir | ||
_, acDg2, _, _ := e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: out}, | ||
cmd.ExecRoot = inputRoot | ||
_, acDg2, _, _ := e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: "out2"}, | ||
fakes.StdOut("stdout2"), fakes.StdErr("stderr2")) | ||
if diff := cmp.Diff(acDg, acDg2); diff == "" { | ||
t.Fatalf("expected action digest to change after input change, got %v\n", acDg) | ||
} | ||
oe = outerr.NewRecordingOutErr() | ||
if _, err := client.ExecuteAction(context.Background(), acDg2.String(), "", tmpDir, oe); err != nil { | ||
t.Errorf("error executeAction: %v", err) | ||
|
@@ -191,22 +245,22 @@ | |
if err != nil { | ||
t.Fatalf("Unable to read downloaded output %v: %v", fp, err) | ||
} | ||
if string(c) != out { | ||
t.Fatalf("Incorrect content in downloaded file %v, want %s, got %s", fp, out, c) | ||
if string(c) != "out2" { | ||
t.Fatalf("Incorrect content in downloaded file %v, want \"out2\", got %s", fp, c) | ||
} | ||
if string(oe.Stderr()) != "stderr2" { | ||
t.Errorf("Incorrect stderr %v, expected \"stderr\"", oe.Stderr()) | ||
t.Errorf("Incorrect stderr %v, expected \"stderr2\"", oe.Stderr()) | ||
} | ||
if string(oe.Stdout()) != "stdout2" { | ||
t.Errorf("Incorrect stdout %v, expected \"stdout\"", oe.Stdout()) | ||
t.Errorf("Incorrect stdout %v, expected \"stdout2\"", oe.Stdout()) | ||
} | ||
} | ||
|
||
func TestTool_ExecuteActionFromRoot(t *testing.T) { | ||
e, cleanup := fakes.NewTestEnv(t) | ||
defer cleanup() | ||
cmd := &command.Command{ | ||
Args: []string{"foo bar baz"}, | ||
Args: []string{"foo", "bar", "baz"}, | ||
ExecRoot: e.ExecRoot, | ||
InputSpec: &command.InputSpec{Inputs: []string{"i1", "i2"}}, | ||
OutputFiles: []string{"a/b/out"}, | ||
|
@@ -218,9 +272,8 @@ | |
if err := os.WriteFile(filepath.Join(e.ExecRoot, "i2"), []byte("i2"), 0644); err != nil { | ||
t.Fatalf("failed creating input file: %v", err) | ||
} | ||
out := "output" | ||
opt := &command.ExecutionOptions{AcceptCached: false, DownloadOutputs: false, DownloadOutErr: true} | ||
e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: out}, | ||
e.Set(cmd, opt, &command.Result{Status: command.SuccessResultStatus}, &fakes.OutputFile{Path: "a/b/out", Contents: "out"}, | ||
fakes.StdOut("stdout"), fakes.StdErr("stderr")) | ||
|
||
client := &Client{GrpcClient: e.Client.GrpcClient} | ||
|
@@ -233,13 +286,17 @@ | |
if err := os.WriteFile(filepath.Join(e.ExecRoot, "input", "i2"), []byte("i2"), 0644); err != nil { | ||
t.Fatalf("failed creating input file: %v", err) | ||
} | ||
if err := os.WriteFile(filepath.Join(e.ExecRoot, "cmd.textproto"), []byte(`arguments: "foo bar baz" | ||
output_files: "a/b/out"`), 0644); err != nil { | ||
t.Fatalf("failed creating command file: %v", err) | ||
reCmdPb := &repb.Command{ | ||
Arguments: []string{"foo", "bar", "baz"}, | ||
OutputFiles: []string{"a/b/out"}, | ||
} | ||
if err := os.WriteFile(filepath.Join(e.ExecRoot, "ac.textproto"), []byte(""), 0644); err != nil { | ||
if err := os.WriteFile(filepath.Join(e.ExecRoot, "cmd.textproto"), []byte(prototext.Format(reCmdPb)), 0644); err != nil { | ||
t.Fatalf("failed creating command file: %v", err) | ||
} | ||
// The tool will embed the Command proto digest into the Action proto, so the `command_digest` field is effectively ignored: | ||
if err := os.WriteFile(filepath.Join(e.ExecRoot, "ac.textproto"), []byte(`command_digest: {hash: "whatever"}`), 0644); err != nil { | ||
t.Fatalf("failed creating action file: %v", err) | ||
} | ||
if _, err := client.ExecuteAction(context.Background(), "", e.ExecRoot, "", oe); err != nil { | ||
t.Errorf("error executeAction: %v", err) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's from #473