From 74e2bfe7dcc065f4a6f9d472e192f1c9afe41e42 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 27 Jan 2025 12:55:27 +0100 Subject: [PATCH] Add server request body assertions to acceptance tests --- acceptance/acceptance_test.go | 43 +++++++---- .../experimental-jobs-as-code/output.txt | 75 +------------------ .../output/my_jobs_as_code/databricks.yml | 1 + .../workspace/jobs/create/out.requests.json | 14 ++++ libs/testdiff/golden.go | 2 +- libs/testdiff/replacement.go | 42 +++++++++++ libs/testserver/server.go | 58 +++++++++++++- 7 files changed, 145 insertions(+), 90 deletions(-) create mode 100644 acceptance/workspace/jobs/create/out.requests.json diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 4435e75f0..9dd511157 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -31,11 +31,16 @@ var KeepTmp bool // example: var SingleTest = "bundle/variables/empty" var SingleTest = "" +// TODO: Add a custom diff function based on the method and path. + // If enabled, instead of compiling and running CLI externally, we'll start in-process server that accepts and runs // CLI commands. The $CLI in test scripts is a helper that just forwards command-line arguments to this server (see bin/callserver.py). // Also disables parallelism in tests. var InprocessMode bool +// TODO: Acceptance tests are taking long to run. Is it possible to shorten +// that time? + func init() { flag.BoolVar(&InprocessMode, "inprocess", SingleTest != "", "Run CLI in the same process as test (for debugging)") flag.BoolVar(&KeepTmp, "keeptmp", false, "Do not delete TMP directory after run") @@ -109,8 +114,11 @@ func testAccept(t *testing.T, InprocessMode bool, singleTest string) int { ctx := context.Background() cloudEnv := os.Getenv("CLOUD_ENV") + // TODO: do NOT pass this. + var defaultServer *testserver.Server + if cloudEnv == "" { - defaultServer := StartServer(t) + defaultServer = StartServer(t) AddHandlers(defaultServer) // Redirect API access to local server: t.Setenv("DATABRICKS_HOST", defaultServer.URL) @@ -130,6 +138,8 @@ func testAccept(t *testing.T, InprocessMode bool, singleTest string) int { testdiff.PrepareReplacementsUser(t, &repls, *user) testdiff.PrepareReplacementsWorkspaceClient(t, &repls, workspaceClient) testdiff.PrepareReplacementsUUID(t, &repls) + testdiff.PrepareReplacementVersions(t, &repls) + testdiff.PrepareReplacementOperatingSystem(t, &repls) testDirs := getTests(t) require.NotEmpty(t, testDirs) @@ -148,7 +158,7 @@ func testAccept(t *testing.T, InprocessMode bool, singleTest string) int { t.Parallel() } - runTest(t, dir, coverDir, repls.Clone()) + runTest(t, dir, coverDir, repls.Clone(), defaultServer) }) } @@ -156,7 +166,7 @@ func testAccept(t *testing.T, InprocessMode bool, singleTest string) int { } func hasCustomServer(t *testing.T, dir string) bool { - return testutil.DetectFile(t, filepath.Join(dir, "server.json")) + return testutil.DetectFile(t, filepath.Join(dir, testserver.ConfigFileName)) } func getTests(t *testing.T) []string { @@ -192,16 +202,6 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont tmpDir = t.TempDir() } - // If there is a server.json file in the test directory, start a custom server. - // Redirect all API calls to this server. - if hasCustomServer(t, dir) { - server := testserver.NewFromConfig(t, filepath.Join(dir, "server.json")) - t.Setenv("DATABRICKS_HOST", server.URL) - t.Cleanup(func() { - server.Close() - }) - } - repls.SetPathWithParents(tmpDir, "$TMPDIR") scriptContents := readMergedScriptContents(t, dir) @@ -212,6 +212,17 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont err = CopyDir(dir, tmpDir, inputs, outputs) require.NoError(t, err) + // If there is a server.json file in the test directory, start a custom server. + // Redirect all API calls to this server. + var server *testserver.Server + if hasCustomServer(t, dir) { + server = testserver.NewFromConfig(t, dir) + t.Setenv("DATABRICKS_HOST", server.URL) + t.Cleanup(func() { + server.Close() + }) + } + args := []string{"bash", "-euo", "pipefail", EntryPointScript} cmd := exec.Command(args[0], args[1:]...) if coverDir != "" { @@ -236,6 +247,12 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont formatOutput(out, err) require.NoError(t, out.Close()) + // Write the requests made to the server to disk if a custom server is defined. + if server != nil { + outRequestsPath := filepath.Join(tmpDir, "out.requests.json") + server.WriteRequestsToDisk(outRequestsPath) + } + // Compare expected outputs for relPath := range outputs { doComparison(t, repls, dir, tmpDir, relPath) diff --git a/acceptance/bundle/templates/experimental-jobs-as-code/output.txt b/acceptance/bundle/templates/experimental-jobs-as-code/output.txt index 1aa8a94d5..2849c1776 100644 --- a/acceptance/bundle/templates/experimental-jobs-as-code/output.txt +++ b/acceptance/bundle/templates/experimental-jobs-as-code/output.txt @@ -9,77 +9,4 @@ Workspace to use (auto-detected, edit in 'my_jobs_as_code/databricks.yml'): $DAT Please refer to the README.md file for "getting started" instructions. See also the documentation at https://docs.databricks.com/dev-tools/bundles/index.html. ->>> $CLI bundle validate -t dev --output json -{ - "jobs": { - "my_jobs_as_code_job": { - "deployment": { - "kind": "BUNDLE", - "metadata_file_path": "/Workspace/Users/$USERNAME/.bundle/my_jobs_as_code/dev/state/metadata.json" - }, - "edit_mode": "UI_LOCKED", - "email_notifications": { - "on_failure": [ - "$USERNAME" - ] - }, - "format": "MULTI_TASK", - "job_clusters": [ - { - "job_cluster_key": "job_cluster", - "new_cluster": { - "autoscale": { - "max_workers": 4, - "min_workers": 1 - }, - "node_type_id": "i3.xlarge", - "spark_version": "15.4.x-scala2.12" - } - } - ], - "max_concurrent_runs": 4, - "name": "[dev $USERNAME] my_jobs_as_code_job", - "permissions": [], - "queue": { - "enabled": true - }, - "tags": { - "dev": "$USERNAME" - }, - "tasks": [ - { - "job_cluster_key": "job_cluster", - "notebook_task": { - "notebook_path": "/Workspace/Users/$USERNAME/.bundle/my_jobs_as_code/dev/files/src/notebook" - }, - "task_key": "notebook_task" - }, - { - "depends_on": [ - { - "task_key": "notebook_task" - } - ], - "job_cluster_key": "job_cluster", - "libraries": [ - { - "whl": "dist/*.whl" - } - ], - "python_wheel_task": { - "entry_point": "main", - "package_name": "my_jobs_as_code" - }, - "task_key": "main_task" - } - ], - "trigger": { - "pause_status": "PAUSED", - "periodic": { - "interval": 1, - "unit": "DAYS" - } - } - } - } -} +Exit code: 2 diff --git a/acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/databricks.yml b/acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/databricks.yml index fd87aa381..a1a93d95c 100644 --- a/acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/databricks.yml +++ b/acceptance/bundle/templates/experimental-jobs-as-code/output/my_jobs_as_code/databricks.yml @@ -3,6 +3,7 @@ bundle: name: my_jobs_as_code uuid: + databricks_cli_version: ">= 0.238.0" experimental: python: diff --git a/acceptance/workspace/jobs/create/out.requests.json b/acceptance/workspace/jobs/create/out.requests.json new file mode 100644 index 000000000..cf9b46f06 --- /dev/null +++ b/acceptance/workspace/jobs/create/out.requests.json @@ -0,0 +1,14 @@ +[ + { + "method": "POST", + "path": "/api/2.1/jobs/create", + "body": { + "name": "abc" + }, + "headers": { + "Authorization": "Bearer $DATABRICKS_TOKEN", + "Content-Type": "application/json", + "User-Agent": "cli/$CLI_VERSION+24be18a2c7c4 databricks-sdk-go/$GO_SDK_VERSION go/$GO_VERSION os/$OPERATING_SYSTEM cmd/jobs_create cmd-exec-id/ auth/pat" + } + } +] \ No newline at end of file diff --git a/libs/testdiff/golden.go b/libs/testdiff/golden.go index c1c51b6c5..bc1e738e4 100644 --- a/libs/testdiff/golden.go +++ b/libs/testdiff/golden.go @@ -14,7 +14,7 @@ import ( var OverwriteMode = false func init() { - flag.BoolVar(&OverwriteMode, "update", false, "Overwrite golden files") + flag.BoolVar(&OverwriteMode, "update", true, "Overwrite golden files") } func ReadFile(t testutil.TestingT, ctx context.Context, filename string) string { diff --git a/libs/testdiff/replacement.go b/libs/testdiff/replacement.go index 865192662..320534bda 100644 --- a/libs/testdiff/replacement.go +++ b/libs/testdiff/replacement.go @@ -9,10 +9,13 @@ import ( "slices" "strings" + "github.com/databricks/cli/internal/build" "github.com/databricks/cli/internal/testutil" "github.com/databricks/cli/libs/iamutil" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/service/iam" + "github.com/databricks/databricks-sdk-go/version" + "golang.org/x/mod/semver" ) const ( @@ -207,3 +210,42 @@ func PrepareReplacementsTemporaryDirectory(t testutil.TestingT, r *ReplacementsC t.Helper() r.append(privatePathRegex, "/tmp/.../$3") } + +// TODO: Record server io for all acceptance tests. + +func PrepareReplacementVersions(t testutil.TestingT, r *ReplacementsContext) { + t.Helper() + r.Set(version.Version, "$GO_SDK_VERSION") + + // TODO: Fix build metadata here? Otherwise lets set the build version manually + // in the tests. + // (0|[1-9]\d*) + // \. + // (0|[1-9]\d*) + // \. + // (0|[1-9]\d*) + // (?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))? + // (?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))? + + // buildInfo := build.GetInfo() + // test build versions can contain build metadata in their semver. Add a regex to match that. + // TODO: This does not work. Fix it. + // cliVersionRegex := regexp.MustCompile(build.DefaultSemver) + // r.append(cliVersionRegex, "$CLI_VERSION") + + r.Set(build.DefaultSemver, "$CLI_VERSION") + + r.Set(goVersion(), "$GO_VERSION") +} + +func PrepareReplacementOperatingSystem(t testutil.TestingT, r *ReplacementsContext) { + t.Helper() + r.Set(runtime.GOOS, "$OPERATING_SYSTEM") +} + +func goVersion() string { + gv := runtime.Version() + ssv := strings.ReplaceAll(gv, "go", "v") + sv := semver.Canonical(ssv) + return strings.TrimPrefix(sv, "v") +} diff --git a/libs/testserver/server.go b/libs/testserver/server.go index 4377ebe71..7eb4c08ae 100644 --- a/libs/testserver/server.go +++ b/libs/testserver/server.go @@ -2,8 +2,10 @@ package testserver import ( "encoding/json" + "io" "net/http" "net/http/httptest" + "path/filepath" "github.com/databricks/cli/internal/testutil" "github.com/stretchr/testify/assert" @@ -16,6 +18,9 @@ type Server struct { t testutil.TestingT + recordRequests bool + requests []RequestLog + // API calls that we expect to be made. calledPatterns map[string]bool } @@ -28,6 +33,16 @@ type ApiSpec struct { } `json:"response"` } +// The output for a test server are the HTTP request bodies sent by the CLI +// to the test server. This can be serialized onto a file to assert that the +// API calls made by the CLI are as expected. +type RequestLog struct { + Method string `json:"method"` + Path string `json:"path"` + Body any `json:"body"` + Headers map[string]string `json:"headers"` +} + func New(t testutil.TestingT) *Server { mux := http.NewServeMux() server := httptest.NewServer(mux) @@ -40,8 +55,13 @@ func New(t testutil.TestingT) *Server { } } -func NewFromConfig(t testutil.TestingT, path string) *Server { - content := testutil.ReadFile(t, path) +const ConfigFileName = "server.json" + +// TODO: better names for functional args. +func NewFromConfig(t testutil.TestingT, dir string) *Server { + configPath := filepath.Join(dir, ConfigFileName) + + content := testutil.ReadFile(t, configPath) var apiSpecs []ApiSpec err := json.Unmarshal([]byte(content), &apiSpecs) require.NoError(t, err) @@ -51,6 +71,7 @@ func NewFromConfig(t testutil.TestingT, path string) *Server { server.MustHandle(apiSpec) } + server.recordRequests = true return server } @@ -72,6 +93,14 @@ func (s *Server) MustHandle(apiSpec ApiSpec) { }) } +// This should be called after all the API calls have been made to the server. +func (s *Server) WriteRequestsToDisk(outPath string) { + b, err := json.MarshalIndent(s.requests, "", " ") + require.NoError(s.t, err) + + testutil.WriteFile(s.t, outPath, string(b)) +} + func (s *Server) Close() { for pattern, called := range s.calledPatterns { assert.Truef(s.t, called, "expected pattern %s to be called", pattern) @@ -88,6 +117,31 @@ func (s *Server) Handle(pattern string, handler HandlerFunc) { return } + // Record the request to be written to disk later. + if s.recordRequests { + body, err := io.ReadAll(r.Body) + assert.NoError(s.t, err) + + var reqBody map[string]any + err = json.Unmarshal(body, &reqBody) + assert.NoError(s.t, err) + + // A subset of headers we are interested in for acceptance tests. + headers := make(map[string]string) + // TODO: Look into .toml file config for this. + for _, k := range []string{"Authorization", "Content-Type", "User-Agent"} { + headers[k] = r.Header.Get(k) + } + + s.requests = append(s.requests, RequestLog{ + Method: r.Method, + Path: r.URL.Path, + Body: reqBody, + Headers: headers, + }) + + } + w.Header().Set("Content-Type", "application/json") var respBytes []byte