From 23ddee8023bae3d069d2097f518905f4bb217fa8 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 17 Dec 2024 18:16:58 +0100 Subject: [PATCH 1/3] Skip job runs during integration testing for PRs (#2024) ## Changes A small subset of tests trigger cluster creation to run jobs. These tests comprise a substantial amount of the total integration test runtime. We can skip them on PRs and only run them on the main branch. ## Tests Confirmed the short runtime is ~20 mins. --- Makefile | 11 ++++++++--- integration/bundle/clusters_test.go | 5 +++++ integration/bundle/python_wheel_test.go | 5 +++++ integration/bundle/spark_jar_test.go | 5 +++++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 6c849986e..a7c0584fa 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,12 @@ vendor: @echo "✓ Filling vendor folder with library code ..." @go mod vendor -integration: - gotestsum --format github-actions --rerun-fails --jsonfile output.json --packages "./integration/..." -- -parallel 4 -timeout=2h +INTEGRATION = gotestsum --format github-actions --rerun-fails --jsonfile output.json --packages "./integration/..." -- -parallel 4 -timeout=2h -.PHONY: lint lintcheck test testonly coverage build snapshot vendor integration +integration: + $(INTEGRATION) + +integration-short: + $(INTEGRATION) -short + +.PHONY: lint lintcheck test testonly coverage build snapshot vendor integration integration-short diff --git a/integration/bundle/clusters_test.go b/integration/bundle/clusters_test.go index f9e5c9b64..c153b8733 100644 --- a/integration/bundle/clusters_test.go +++ b/integration/bundle/clusters_test.go @@ -44,6 +44,11 @@ func TestDeployBundleWithCluster(t *testing.T) { require.NoError(t, err) require.NotNil(t, cluster) + if testing.Short() { + t.Log("Skip the job run in short mode") + return + } + out, err := runResource(t, ctx, root, "foo") require.NoError(t, err) require.Contains(t, out, "Hello World!") diff --git a/integration/bundle/python_wheel_test.go b/integration/bundle/python_wheel_test.go index 5cca1cb53..f7fb1ff3f 100644 --- a/integration/bundle/python_wheel_test.go +++ b/integration/bundle/python_wheel_test.go @@ -29,6 +29,11 @@ func runPythonWheelTest(t *testing.T, templateName, sparkVersion string, pythonW destroyBundle(t, ctx, bundleRoot) }) + if testing.Short() { + t.Log("Skip the job run in short mode") + return + } + out, err := runResource(t, ctx, bundleRoot, "some_other_job") require.NoError(t, err) require.Contains(t, out, "Hello from my func") diff --git a/integration/bundle/spark_jar_test.go b/integration/bundle/spark_jar_test.go index 65893052e..cbdf5a00c 100644 --- a/integration/bundle/spark_jar_test.go +++ b/integration/bundle/spark_jar_test.go @@ -30,6 +30,11 @@ func runSparkJarTestCommon(t *testing.T, ctx context.Context, sparkVersion, arti destroyBundle(t, ctx, bundleRoot) }) + if testing.Short() { + t.Log("Skip the job run in short mode") + return + } + out, err := runResource(t, ctx, bundleRoot, "jar_job") require.NoError(t, err) require.Contains(t, out, "Hello from Jar!") From 13fa43e0f5af0c2b0d5a6397ca7273a36548cbc1 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 17 Dec 2024 18:34:09 +0100 Subject: [PATCH 2/3] Remove superfluous helper (#2028) ## Changes There was only one helper for AWS and not the other clouds. Found this when looking through double calls to `acc.WorkspaceTest()` (see `TestPythonWheelTaskDeployAndRunOnInteractiveCluster`). ## Tests n/a --- integration/bundle/clusters_test.go | 6 +++--- integration/bundle/python_wheel_test.go | 4 +--- internal/testutil/cloud.go | 4 ---- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/integration/bundle/clusters_test.go b/integration/bundle/clusters_test.go index c153b8733..449206208 100644 --- a/integration/bundle/clusters_test.go +++ b/integration/bundle/clusters_test.go @@ -12,12 +12,12 @@ import ( ) func TestDeployBundleWithCluster(t *testing.T) { - ctx, wt := acc.WorkspaceTest(t) - - if testutil.IsAWSCloud(wt) { + if testutil.GetCloud(t) == testutil.AWS { t.Skip("Skipping test for AWS cloud because it is not permitted to create clusters") } + ctx, wt := acc.WorkspaceTest(t) + nodeTypeId := testutil.GetCloud(t).NodeTypeID() uniqueId := uuid.New().String() root := initTestTemplate(t, ctx, "clusters", map[string]any{ diff --git a/integration/bundle/python_wheel_test.go b/integration/bundle/python_wheel_test.go index f7fb1ff3f..62846f7b5 100644 --- a/integration/bundle/python_wheel_test.go +++ b/integration/bundle/python_wheel_test.go @@ -56,9 +56,7 @@ func TestPythonWheelTaskDeployAndRunWithWrapper(t *testing.T) { } func TestPythonWheelTaskDeployAndRunOnInteractiveCluster(t *testing.T) { - _, wt := acc.WorkspaceTest(t) - - if testutil.IsAWSCloud(wt) { + if testutil.GetCloud(t) == testutil.AWS { t.Skip("Skipping test for AWS cloud because it is not permitted to create clusters") } diff --git a/internal/testutil/cloud.go b/internal/testutil/cloud.go index 4a8a89645..33921db0c 100644 --- a/internal/testutil/cloud.go +++ b/internal/testutil/cloud.go @@ -58,7 +58,3 @@ func GetCloud(t TestingT) Cloud { } return -1 } - -func IsAWSCloud(t TestingT) bool { - return GetCloud(t) == AWS -} From 5b84856b17178bd802fb58ea1256372d960aa845 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 17 Dec 2024 20:05:42 +0100 Subject: [PATCH 3/3] Correctly handle required query params in CLI generation (#2027) ## Changes If there's required query params, it is a top-level field of request object and not a field of nested request body. This is needed for upcoming changes from OpenAPI spec changes where such query parameters is introduced. No changes after regenerating CLI with current spec and the fix (appears we haven't had such params before) --- .codegen/service.go.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.codegen/service.go.tmpl b/.codegen/service.go.tmpl index ef7977e1b..ee2c7b0fd 100644 --- a/.codegen/service.go.tmpl +++ b/.codegen/service.go.tmpl @@ -411,5 +411,5 @@ func new{{.PascalName}}() *cobra.Command { {{- define "request-body-obj" -}} {{- $method := .Method -}} {{- $field := .Field -}} - {{$method.CamelName}}Req{{ if (and $method.RequestBodyField (not $field.IsPath)) }}.{{$method.RequestBodyField.PascalName}}{{end}}.{{$field.PascalName}} + {{$method.CamelName}}Req{{ if (and $method.RequestBodyField (and (not $field.IsPath) (not $field.IsQuery))) }}.{{$method.RequestBodyField.PascalName}}{{end}}.{{$field.PascalName}} {{- end -}}