zeroshade commented on code in PR #264:
URL: https://github.com/apache/iceberg-go/pull/264#discussion_r1925603606
##########
catalog/registry.go:
##########
@@ -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 loo kup the catalog by checking the
scheme. Again,
Review Comment:
got a typo here :)
##########
catalog/registry_test.go:
##########
@@ -88,6 +88,76 @@ func TestCatalogRegistry(t *testing.T) {
}, catalog.GetRegisteredCatalogs())
}
+func TestRegistryPanic(t *testing.T) {
+ defer func() {
+ if r := recover(); r != nil {
+ assert.Equal(t, "catalog: RegisterCatalog catalog
factory is nil", r)
+ } else {
+ t.Fatalf("expect a panic but did not get one")
+ }
+
+ }()
+ catalog.Register("foobar", nil)
+ t.Fatalf("expect a panic but Register function did not")
Review Comment:
`assert.PanicsWithValue(t, "catalog: RegisterCatalog catalog factory is
nil", func() { catalog.Register("foobar", nil) })`
cleans this up and doesn't require you to manually do the defer etc.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
