This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new e83d013 ci(catalog): Improvement to unittest cases in
catalog/registry.go (#264)
e83d013 is described below
commit e83d0136a88b3882ab3e3207ee4ef19da9587076
Author: Dao Thanh Tung <[email protected]>
AuthorDate: Wed Jan 22 18:30:36 2025 +0000
ci(catalog): Improvement to unittest cases in catalog/registry.go (#264)
Hi team,
I'm new to iceberg and iceberg-go and working on the adoption at my
current company. I was digging at the code and found some area to
contribute to. Not saying achiving 100% code coverage is always good but
it serves as an entry point for my understanding of the intenals :bow:
---------
Signed-off-by: dttung2905 <[email protected]>
---
catalog/registry.go | 14 +++++------
catalog/registry_test.go | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/catalog/registry.go b/catalog/registry.go
index 544d419..e8e6132 100644
--- a/catalog/registry.go
+++ b/catalog/registry.go
@@ -86,7 +86,7 @@ func Unregister(catalogType string) {
defaultRegistry.remove(catalogType)
}
-// GetRegsisteredCatalogs returns the list of registered catalog names that can
+// GetRegisteredCatalogs returns the list of registered catalog names that can
// be looked up via LoadCatalog.
func GetRegisteredCatalogs() []string {
return defaultRegistry.getKeys()
@@ -98,7 +98,7 @@ func GetRegisteredCatalogs() []string {
// easier catalog loading but also to allow for custom catalog implementations
to
// be registered and loaded external to this module.
//
-// The name parameter is used to lookup the catalog configuration, if one
exists,
+// The name parameter is used to look up the catalog configuration, if one
exists,
// that was loaded from the configuration file, ".iceberg-go.yaml". By default,
// the config file is loaded from the user's home directory, but the directory
can
// be changed by setting the GOICEBERG_HOME environment variable to the path
of the
@@ -110,20 +110,20 @@ func GetRegisteredCatalogs() []string {
// priority over any loaded config.
//
// If there is no "type" in the configuration and no "type" in the passed in
properties,
-// then the "uri" property is used to lookup the catalog by checking the
scheme. Again,
-// if there is a "uri" key set in the passed in "props" it will take priority
over the
+// then the "uri" property is used to look up the catalog by checking the
scheme. Again,
+// if there is an "uri" key set in the passed in "props" it will take priority
over the
// configuration.
//
-// Currently the following catalog types are registered by default:
+// Currently, the following catalog types are registered by default:
//
// - "glue" for AWS Glue Data Catalog, the rest of the URI is ignored, all
configuration
// should be provided using the properties. "glue.region", "glue.endpoint",
// "glue.max-retries", etc. Default AWS credentials are used if found, or
can be
// overridden by setting "glue.access-key-id", "glue.secret-access-key",
and "glue.session-token".
//
-// - "rest" for a REST API catalog, if the properties have a "uri" key, then
that will be used
+// - "rest" for a REST API catalog, if the properties have an "uri" key,
then that will be used
// as the REST endpoint, otherwise the URI is used as the endpoint. The
REST catalog also
-// registers "http" and "https" so that Load with an http/s URI will
automatically
+// registers "http" and "https" so that Load with a http/s URI will
automatically
// load the REST Catalog.
func Load(name string, props iceberg.Properties) (Catalog, error) {
if name == "" {
diff --git a/catalog/registry_test.go b/catalog/registry_test.go
index 77347a7..9bace89 100644
--- a/catalog/registry_test.go
+++ b/catalog/registry_test.go
@@ -88,6 +88,67 @@ func TestCatalogRegistry(t *testing.T) {
}, catalog.GetRegisteredCatalogs())
}
+func TestRegistryPanic(t *testing.T) {
+ assert.PanicsWithValue(t, "catalog: RegisterCatalog catalog factory is
nil", func() { catalog.Register("foobar", nil) })
+}
+
+func TestCatalogWithEmptyName(t *testing.T) {
+ config.EnvConfig.DefaultCatalog = "test-default"
+ config.EnvConfig.Catalogs = map[string]config.CatalogConfig{
+ "test-default": {
+ URI: "http://localhost:8181/";,
+ Credential: "default-credential",
+ Warehouse: "/default/warehouse",
+ CatalogType: "mock",
+ },
+ }
+ catalog.Register("mock", catalog.RegistrarFunc(func(name string, props
iceberg.Properties) (catalog.Catalog, error) {
+ // Ensure the correct name and properties are passed
+ assert.Equal(t, "test-default", name)
+ assert.Equal(t, "http://localhost:8181/";, props.Get("uri", ""))
+ assert.Equal(t, "default-credential", props.Get("credential",
""))
+ assert.Equal(t, "/default/warehouse", props.Get("warehouse",
""))
+ return nil, nil
+ }))
+ c, err := catalog.Load("", nil)
+ assert.Nil(t, c)
+ assert.NoError(t, err)
+ assert.ElementsMatch(t, []string{
+ "rest",
+ "http",
+ "https",
+ "glue",
+ "mock",
+ }, catalog.GetRegisteredCatalogs())
+ catalog.Unregister("mock")
+
+}
+
+func TestCatalogLoadInvalidURI(t *testing.T) {
+ config.EnvConfig.DefaultCatalog = "default"
+ config.EnvConfig.Catalogs = map[string]config.CatalogConfig{
+ "default": {
+ URI: "http://localhost:8181/";,
+ Credential: "default-credential",
+ Warehouse: "/default/warehouse",
+ CatalogType: "mock",
+ },
+ }
+
+ catalog.Register("mock", catalog.RegistrarFunc(func(name string, props
iceberg.Properties) (catalog.Catalog, error) {
+ return nil, nil
+ }))
+ props := iceberg.Properties{
+ "uri": "://invalid-uri", // This will cause url.Parse to fail
+ }
+ c, err := catalog.Load("mock", props)
+
+ assert.Nil(t, c)
+ assert.Error(t, err)
+ assert.Contains(t, err.Error(), "failed to parse catalog URI")
+ catalog.Unregister("mock")
+}
+
func TestRegistryFromConfig(t *testing.T) {
var params url.Values
