From b672f1a5be5a115cc91c1734c1d94523572f75a0 Mon Sep 17 00:00:00 2001 From: Andre Furlan Date: Wed, 7 Dec 2022 21:47:44 -0800 Subject: [PATCH 1/4] adding integration tests Signed-off-by: Andre Furlan --- .github/workflows/go.yml | 45 ++++++++++++++++++++++++++- Makefile | 6 ++++ integration_tests/integration.go | 1 + integration_tests/integration_test.go | 29 +++++++++++++++++ 4 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 integration_tests/integration.go create mode 100644 integration_tests/integration_test.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 8a2e6854..ac12f0ca 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -1,4 +1,4 @@ -name: Go +name: CI on: push: @@ -19,6 +19,49 @@ jobs: uses: golangci/golangci-lint-action@v2 with: version: v1.48 + + integration: + permissions: + contents: read + packages: read + name: Integration Tests + strategy: + matrix: + go-version: [1.19.x] + os: [ubuntu-latest] + runs-on: ${{ matrix.os }} + services: + test-server: + options: --user "appuser" + credentials: + username: ${{ github.repository }} + password: ${{ secrets.GITHUB_TOKEN }} + image: ghcr.io/databricks/databricks/databricks-thrift-test-server:main + ports: + - 8087:8087 + steps: + - name: Check out code into the Go module directory + uses: actions/checkout@v3 + + - name: Set up Go Toolchain + uses: actions/setup-go@v3 + with: + go-version: ${{ matrix.go-version }} + + - name: Cache Go artifacts + uses: actions/cache@v2 + with: + path: | + ~/go/pkg/mod + ~/.cache/go-build + key: ${{ runner.os }}-go-${{ matrix.go-version }}-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go-${{ matrix.go-version }}- + + + - name: run integration tests + run: make test-integration + build-and-test: name: Test and Build strategy: diff --git a/Makefile b/Makefile index 9c81d88b..c7291e73 100644 --- a/Makefile +++ b/Makefile @@ -57,6 +57,12 @@ test: bin/gotestsum ## Run the go unit tests. @echo "INFO: Running all go unit tests." CGO_ENABLED=0 ./bin/gotestsum --format pkgname-and-test-fails --junitfile $(TEST_RESULTS_DIR)/unit-tests.xml ./... +.PHONY: test-integration +test-integration: bin/gotestsum ## Run the go unit integration tests. + @echo "INFO: Running all go integration tests." + CGO_ENABLED=0 ./bin/gotestsum --format pkgname-and-test-fails -- -tags=integration ./... + + .PHONY: coverage coverage: bin/gotestsum ## Report the unit test code coverage. @echo "INFO: Generating unit test coverage report." diff --git a/integration_tests/integration.go b/integration_tests/integration.go new file mode 100644 index 00000000..6108072f --- /dev/null +++ b/integration_tests/integration.go @@ -0,0 +1 @@ +package integration_tests diff --git a/integration_tests/integration_test.go b/integration_tests/integration_test.go new file mode 100644 index 00000000..e4dddbe6 --- /dev/null +++ b/integration_tests/integration_test.go @@ -0,0 +1,29 @@ +//go:build integration + +package integration_tests + +import ( + "context" + "database/sql" + "testing" + + dbsql "github.com/databricks/databricks-sql-go" + "github.com/stretchr/testify/require" +) + +func TestIntegrationPing(t *testing.T) { + + connector, err := dbsql.NewConnector( + dbsql.WithServerHostname("localhost"), + dbsql.WithPort(8087), + ) + require.NoError(t, err) + db := sql.OpenDB(connector) + defer db.Close() + + t.Run("it can ping", func(t *testing.T) { + if err := db.PingContext(context.Background()); err != nil { + require.NoError(t, err) + } + }) +} From 5047a9149b6e0961a5c6cf36ff59769d54546f08 Mon Sep 17 00:00:00 2001 From: Andre Furlan Date: Fri, 9 Dec 2022 17:57:48 -0800 Subject: [PATCH 2/4] adapt to test server 0.1.0 Signed-off-by: Andre Furlan --- .github/workflows/go.yml | 47 ++++++------------------ connector.go | 17 ++++----- integration_tests/integration_test.go | 52 +++++++++++++++++++++++++-- 3 files changed, 66 insertions(+), 50 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index ac12f0ca..599ffebe 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -20,11 +20,11 @@ jobs: with: version: v1.48 - integration: + build-and-test: + name: Test and Build permissions: contents: read packages: read - name: Integration Tests strategy: matrix: go-version: [1.19.x] @@ -32,44 +32,12 @@ jobs: runs-on: ${{ matrix.os }} services: test-server: - options: --user "appuser" credentials: username: ${{ github.repository }} password: ${{ secrets.GITHUB_TOKEN }} - image: ghcr.io/databricks/databricks/databricks-thrift-test-server:main + image: ghcr.io/databricks/databricks/databricks-thrift-test-server:0.1.0 ports: - 8087:8087 - steps: - - name: Check out code into the Go module directory - uses: actions/checkout@v3 - - - name: Set up Go Toolchain - uses: actions/setup-go@v3 - with: - go-version: ${{ matrix.go-version }} - - - name: Cache Go artifacts - uses: actions/cache@v2 - with: - path: | - ~/go/pkg/mod - ~/.cache/go-build - key: ${{ runner.os }}-go-${{ matrix.go-version }}-${{ hashFiles('**/go.sum') }} - restore-keys: | - ${{ runner.os }}-go-${{ matrix.go-version }}- - - - - name: run integration tests - run: make test-integration - - build-and-test: - name: Test and Build - strategy: - matrix: - go-version: [1.19.x] - os: [ubuntu-latest] - runs-on: ${{ matrix.os }} - steps: - name: Check out code into the Go module directory uses: actions/checkout@v3 @@ -103,10 +71,15 @@ jobs: fi go get -v -t -d ./... + - name: Build + run: make linux + - name: Test run: make test env: CGO_ENABLED: 0 - - name: Build - run: make linux + - name: run integration tests + run: make test-integration + env: + CGO_ENABLED: 0 \ No newline at end of file diff --git a/connector.go b/connector.go index 7b6935cd..9a9a643f 100644 --- a/connector.go +++ b/connector.go @@ -19,13 +19,12 @@ type connector struct { } func (c *connector) Connect(ctx context.Context) (driver.Conn, error) { - var catalogName *cli_service.TIdentifier - var schemaName *cli_service.TIdentifier + var initialNamespace *cli_service.TNamespace if c.cfg.Catalog != "" { - catalogName = cli_service.TIdentifierPtr(cli_service.TIdentifier(c.cfg.Catalog)) + initialNamespace.CatalogName = cli_service.TIdentifierPtr(cli_service.TIdentifier(c.cfg.Catalog)) } if c.cfg.Schema != "" { - schemaName = cli_service.TIdentifierPtr(cli_service.TIdentifier(c.cfg.Schema)) + initialNamespace.SchemaName = cli_service.TIdentifierPtr(cli_service.TIdentifier(c.cfg.Schema)) } tclient, err := client.InitThriftClient(c.cfg) @@ -34,13 +33,11 @@ func (c *connector) Connect(ctx context.Context) (driver.Conn, error) { } // we need to ensure that open session will eventually end + session, err := tclient.OpenSession(ctx, &cli_service.TOpenSessionReq{ - ClientProtocol: c.cfg.ThriftProtocolVersion, - Configuration: make(map[string]string), - InitialNamespace: &cli_service.TNamespace{ - CatalogName: catalogName, - SchemaName: schemaName, - }, + ClientProtocol: c.cfg.ThriftProtocolVersion, + Configuration: make(map[string]string), + InitialNamespace: initialNamespace, CanUseMultipleCatalogs: &c.cfg.CanUseMultipleCatalogs, }) diff --git a/integration_tests/integration_test.go b/integration_tests/integration_test.go index e4dddbe6..925fa13e 100644 --- a/integration_tests/integration_test.go +++ b/integration_tests/integration_test.go @@ -8,6 +8,7 @@ import ( "testing" dbsql "github.com/databricks/databricks-sql-go" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -16,14 +17,59 @@ func TestIntegrationPing(t *testing.T) { connector, err := dbsql.NewConnector( dbsql.WithServerHostname("localhost"), dbsql.WithPort(8087), + dbsql.WithHTTPPath("session"), //test case name + dbsql.WithMaxRows(100), ) require.NoError(t, err) db := sql.OpenDB(connector) defer db.Close() - t.Run("it can ping", func(t *testing.T) { - if err := db.PingContext(context.Background()); err != nil { - require.NoError(t, err) + t.Run("it can query multiple result pages", func(t *testing.T) { + rows, err1 := db.QueryContext(context.Background(), `select * from default.diamonds limit 250`) + require.Nil(t, err1) + require.NotNil(t, rows) + defer rows.Close() + type row struct { + _c0 int + carat float64 + cut string + color string + clarity string + depth sql.NullFloat64 + table sql.NullFloat64 + price int + x float64 + y float64 + z float64 } + expectedColumnNames := []string{"_c0", "carat", "cut", "color", "clarity", "depth", "table", "price", "x", "y", "z"} + expectedDatabaseType := []string{"INT", "DOUBLE", "STRING", "STRING", "STRING", "DOUBLE", "DOUBLE", "INT", "DOUBLE", "DOUBLE", "DOUBLE"} + expectedNullable := []bool{false, false, false, false, false, false, false, false, false, false, false} + + cols, err := rows.Columns() + require.NoError(t, err) + require.Equal(t, expectedColumnNames, cols) + + types, err := rows.ColumnTypes() + require.NoError(t, err) + + for i, v := range types { + assert.Equal(t, expectedColumnNames[i], v.Name()) + assert.Equal(t, expectedDatabaseType[i], v.DatabaseTypeName()) + nullable, ok := v.Nullable() + assert.False(t, ok) + assert.Equal(t, expectedNullable[i], nullable) + } + var allrows []row + for rows.Next() { + // After row 10 this will cause one fetch call, as 10 rows (maxRows config) will come from the first execute statement call. + r := row{} + err := rows.Scan(&r._c0, &r.carat, &r.cut, &r.color, &r.clarity, &r.depth, &r.table, &r.price, &r.x, &r.y, &r.z) + assert.Nil(t, err) + allrows = append(allrows, r) + } + assert.Equal(t, 250, len(allrows)) + assert.Nil(t, rows.Err()) + }) } From 77ac24abb15dc99b2f0d84b89cea2355dbe1b354 Mon Sep 17 00:00:00 2001 From: Andre Furlan Date: Fri, 9 Dec 2022 18:05:16 -0800 Subject: [PATCH 3/4] fix test Signed-off-by: Andre Furlan --- .github/workflows/go.yml | 2 +- connector.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 599ffebe..77352262 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -79,7 +79,7 @@ jobs: env: CGO_ENABLED: 0 - - name: run integration tests + - name: Contract Test run: make test-integration env: CGO_ENABLED: 0 \ No newline at end of file diff --git a/connector.go b/connector.go index 9a9a643f..2f84fc13 100644 --- a/connector.go +++ b/connector.go @@ -20,10 +20,15 @@ type connector struct { func (c *connector) Connect(ctx context.Context) (driver.Conn, error) { var initialNamespace *cli_service.TNamespace + if c.cfg.Catalog != "" { + initialNamespace = &cli_service.TNamespace{} initialNamespace.CatalogName = cli_service.TIdentifierPtr(cli_service.TIdentifier(c.cfg.Catalog)) } if c.cfg.Schema != "" { + if initialNamespace == nil { + initialNamespace = &cli_service.TNamespace{} + } initialNamespace.SchemaName = cli_service.TIdentifierPtr(cli_service.TIdentifier(c.cfg.Schema)) } From 37c73198491dcd3f5334864b893b2086dc297351 Mon Sep 17 00:00:00 2001 From: Andre Furlan Date: Fri, 9 Dec 2022 18:09:47 -0800 Subject: [PATCH 4/4] try again Signed-off-by: Andre Furlan --- connector.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/connector.go b/connector.go index 2f84fc13..e29c1246 100644 --- a/connector.go +++ b/connector.go @@ -19,17 +19,21 @@ type connector struct { } func (c *connector) Connect(ctx context.Context) (driver.Conn, error) { + var catalogName *cli_service.TIdentifier + var schemaName *cli_service.TIdentifier var initialNamespace *cli_service.TNamespace if c.cfg.Catalog != "" { - initialNamespace = &cli_service.TNamespace{} - initialNamespace.CatalogName = cli_service.TIdentifierPtr(cli_service.TIdentifier(c.cfg.Catalog)) + catalogName = cli_service.TIdentifierPtr(cli_service.TIdentifier(c.cfg.Catalog)) } if c.cfg.Schema != "" { - if initialNamespace == nil { - initialNamespace = &cli_service.TNamespace{} + schemaName = cli_service.TIdentifierPtr(cli_service.TIdentifier(c.cfg.Schema)) + } + if catalogName != nil || schemaName != nil { + initialNamespace = &cli_service.TNamespace{ + CatalogName: catalogName, + SchemaName: schemaName, } - initialNamespace.SchemaName = cli_service.TIdentifierPtr(cli_service.TIdentifier(c.cfg.Schema)) } tclient, err := client.InitThriftClient(c.cfg)