shunping commented on code in PR #35903:
URL: https://github.com/apache/beam/pull/35903#discussion_r2292526524


##########
sdks/go/container/boot_test.go:
##########
@@ -205,57 +205,96 @@ func constructArtifactInformation(t *testing.T, roleUrn 
string, path string, sha
        }
 }
 
+func clearEnvVars() {
+       _ = os.Unsetenv(cloudProfilingJobName)
+       _ = os.Unsetenv(cloudProfilingJobID)
+}
+
 func TestConfigureGoogleCloudProfilerEnvVars(t *testing.T) {
        tests := []struct {
-               name          string
-               inputMetadata map[string]string
-               expectedName  string
-               expectedID    string
-               expectedError string
+               name           string
+               options        string
+               metadata       map[string]string
+               expectedName   string
+               expectedID     string
+               expectingError bool
        }{
                {
-                       "nil metadata",
-                       nil,
-                       "",
-                       "",
-                       "enable_google_cloud_profiler is set to true, but no 
metadata is received from provision server, profiling will not be enabled",
-               },
-               {
-                       "missing name",
-                       map[string]string{"job_id": "12345"},
-                       "",
-                       "",
-                       "required job_name missing from metadata, profiling 
will not be enabled without it",
+                       name: "Profiler name from options",
+                       options: `{
+                               "beam:option:go_options:v1": {
+                                       "options": {
+                                               "dataflow_service_options": 
"enable_google_cloud_profiler=custom_profiler"
+                                       }
+                               }
+                       }`,
+                       metadata: map[string]string{
+                               "job_id": "job-123",
+                       },
+                       expectedName:   "custom_profiler",
+                       expectedID:     "job-123",
+                       expectingError: false,
                },
                {
-                       "missing id",
-                       map[string]string{"job_name": "my_job"},
-                       "",
-                       "",
-                       "required job_id missing from metadata, profiling will 
not be enabled without it",
+                       name: "Fallback to job_name",
+                       options: `{
+                               "beam:option:go_options:v1": {
+                                       "options": {
+                                           "dataflow_service_options": 
"enable_google_cloud_profiler"
+                                       }
+                               }
+                       }`,
+                       metadata: map[string]string{
+                               "job_name": "fallback_profiler",
+                               "job_id":   "job-456",
+                       },
+                       expectedName:   "fallback_profiler",
+                       expectedID:     "job-456",
+                       expectingError: false,
                },
                {
-                       "correct",
-                       map[string]string{"job_name": "my_job", "job_id": "42"},
-                       "my_job",
-                       "42",
-                       "",
+                       name: "Missing job_id",

Review Comment:
   Other than the missing job name error, can we also test the case when both 
profiler name and job name are missing?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@beam.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to