[ 
https://issues.apache.org/jira/browse/BEAM-14532?focusedWorklogId=777673&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-777673
 ]

ASF GitHub Bot logged work on BEAM-14532:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 02/Jun/22 16:14
            Start Date: 02/Jun/22 16:14
    Worklog Time Spent: 10m 
      Work Description: msbukal commented on code in PR #17803:
URL: https://github.com/apache/beam/pull/17803#discussion_r888142695


##########
sdks/go/test/integration/io/fhirio/fhirio.go:
##########
@@ -0,0 +1,30 @@
+package fhirio
+
+import (
+       "strings"
+
+       "github.com/apache/beam/sdks/v2/go/pkg/beam"
+       "github.com/apache/beam/sdks/v2/go/pkg/beam/io/fhirio"
+       "github.com/apache/beam/sdks/v2/go/pkg/beam/testing/passert"
+       "github.com/apache/beam/sdks/v2/go/pkg/beam/testing/ptest"
+)
+
+func ReadPipeline(testResourcePaths []string) *beam.Pipeline {

Review Comment:
   Go style usually actively disapproves of running test logic in a separate, 
non-test method. Is there a reason you do a pattern like this? 
   
   Otherwise it should just go in TestRead_*



##########
sdks/go/pkg/beam/io/fhirio/read_test.go:
##########
@@ -76,15 +76,20 @@ func TestRead(t *testing.T) {
                        })
                        pipelineResult := ptest.RunAndValidate(t, p)
                        counterResults := 
pipelineResult.Metrics().AllMetrics().Counters()
-                       if len(counterResults) != 1 {
-                               t.Fatal("Only one counter should have been 
used")
-                       }
-                       if counterResults[0].Name() != 
"fhirio/read_resource_error_count" {
-                               t.Fatal("Only error counter should have been 
used")
-                       }
-                       if counterResults[0].Result() != 
int64(len(testResourcePaths)) {
-                               t.Fatal("Counter should have been incremented 
by the number of test resource paths")
+                       if counterResult := counterResults[0]; 
len(counterResults) == 1 {

Review Comment:
   go style is more like the left, you can just move up the assignment like
   
   ```
   counterResult := counterResults[0]
   if len(counterResults) != 1 {
       t.Fatalf(...)
   }
   [other validation]
   ```





Issue Time Tracking
-------------------

    Worklog Id:     (was: 777673)
    Time Spent: 1.5h  (was: 1h 20m)

> Add integration test for Go FhirIO Read transform
> -------------------------------------------------
>
>                 Key: BEAM-14532
>                 URL: https://issues.apache.org/jira/browse/BEAM-14532
>             Project: Beam
>          Issue Type: Task
>          Components: io-go-gcp
>            Reporter: Lucas Nogueira
>            Assignee: Lucas Nogueira
>            Priority: P2
>          Time Spent: 1.5h
>  Remaining Estimate: 0h
>
> This ticket is a follow up to 
> https://issues.apache.org/jira/browse/BEAM-14451 and it consists of adding 
> integration testing to the Read transform added on the mentioned ticket.
> Buganizer ticket: https://b.corp.google.com/issues/234357022/



--
This message was sent by Atlassian Jira
(v8.20.7#820007)

Reply via email to