lostluck commented on a change in pull request #16903:
URL: https://github.com/apache/beam/pull/16903#discussion_r814344572



##########
File path: sdks/go/pkg/beam/runners/dataflow/dataflow_test.go
##########
@@ -27,3 +32,129 @@ func TestDontUseFlagAsPipelineOption(t *testing.T) {
                t.Fatalf("%q should be in the filter, but isn't set", f)
        }
 }
+
+func TestGetJobOptions(t *testing.T) {
+       *labels = `{"label1": "val1", "label2": "val2"}`
+       *stagingLocation = "gs://testStagingLocation"
+       *autoscalingAlgorithm = "NONE"
+       *minCPUPlatform = "testPlatform"
+
+       *gcpopts.Project = "testProject"
+       *gcpopts.Region = "testRegion"
+
+       *jobopts.Experiments = "use_runner_v2,use_portable_job_submission"
+       *jobopts.JobName = "testJob"
+
+       opts, err := getJobOptions(nil)
+       if err != nil {
+               t.Fatalf("getJobOptions() returned error %q, want %q", err, 
"nil")
+       }
+       if got, want := opts.Name, "testJob"; got != want {
+               t.Fatalf("getJobOptions().Name = %q, want %q", got, want)
+       }
+       if got, want := len(opts.Experiments), 3; got != want {
+               t.Fatalf("len(getJobOptions().Experiments) = %q, want %q", got, 
want)
+       }
+       sort.Strings(opts.Experiments)
+       expectedExperiments := []string{"min_cpu_platform=testPlatform", 
"use_portable_job_submission", "use_runner_v2"}
+       for i := 0; i < 3; i++ {
+               if got, want := opts.Experiments[i], expectedExperiments[i]; 
got != want {
+                       t.Fatalf("getJobOptions().Experiments = %q, want %q", 
got, want)
+               }
+       }
+       if got, want := opts.Project, "testProject"; got != want {
+               t.Fatalf("getJobOptions().Project = %q, want %q", got, want)
+       }
+       if got, want := opts.Region, "testRegion"; got != want {
+               t.Fatalf("getJobOptions().Region = %q, want %q", got, want)
+       }
+       if got, want := len(opts.Labels), 2; got != want {
+               t.Fatalf("len(getJobOptions().Labels) = %q, want %q", got, want)
+       }
+       if got, want := opts.Labels["label1"], "val1"; got != want {
+               t.Fatalf("getJobOptions().Labels[\"label1\"] = %q, want %q", 
got, want)
+       }
+       if got, want := opts.Labels["label2"], "val2"; got != want {
+               t.Fatalf("getJobOptions().Labels[\"label2\"] = %q, want %q", 
got, want)
+       }
+       if got, want := opts.TempLocation, "gs://testStagingLocation/tmp"; got 
!= want {
+               t.Fatalf("getJobOptions().TempLocation = %q, want %q", got, 
want)

Review comment:
       nit: all of these can be t.Errorf instead, so that one can get more 
error information from a single run, rather than needing to fix each one at a 
time.
   
   t.Fatal should be reserved for when the test is unable to continue, or when 
the cases are isolated in their own t.Runs.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to