lliangyu-lin commented on code in PR #400:
URL: https://github.com/apache/iceberg-go/pull/400#discussion_r2060881839


##########
catalog/glue/glue_test.go:
##########
@@ -791,7 +791,7 @@ func TestGlueListTablesIntegration(t *testing.T) {
        }
 
        assert.NoError(lastErr)
-       assert.Equal([]string{os.Getenv("TEST_DATABASE_NAME"), 
os.Getenv("TEST_TABLE_NAME")}, tbls[1])
+       assert.Contains(tbls, []string{os.Getenv("TEST_DATABASE_NAME"), 
os.Getenv("TEST_TABLE_NAME")})

Review Comment:
   Hmmm, I'm confused on the order part.
   Right now this list table integration test essentially list all the tables 
in the input database, and check whether the input table name is part of the 
listed tables by appending all table ids to an array.
   ```
   tbls := make([]table.Identifier, 0)
        for tbl, err := range iter {
                tbls = append(tbls, tbl)
   ```          
    But then checking assert with second element `tbls[1]` will cause out of 
bound, and there's no guarantee it's the second element anyway. So I used 
`contain` here, but we could also just do comparison right within the loop.



-- 
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: issues-unsubscr...@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to