laskoviymishka commented on code in PR #1543:
URL: https://github.com/apache/iceberg-go/pull/1543#discussion_r3678316500
##########
view/view_test.go:
##########
@@ -145,3 +145,15 @@ func TestCreateViewReturnsMetadataCloseError(t *testing.T)
{
require.EqualError(t, err, "error on close")
require.Nil(t, createdView)
}
+
+func (t *ViewTestSuite) TestIdentifierReturnsDefensiveCopy() {
+ identifier := []string{"namespace", "view"}
+ vw := New(identifier, nil, "metadata.json")
Review Comment:
the `nil` `Metadata` is safe today since we only touch `Identifier()`, but
it's a sharp edge — any future assertion that reaches into the metadata
(equality, `CurrentVersion()`) nil-derefs with an opaque panic.
I'd either build a minimal `Metadata` or drop a one-line comment that nil is
intentional because this test only exercises identifier isolation. wdyt?
##########
view/view_test.go:
##########
@@ -145,3 +145,15 @@ func TestCreateViewReturnsMetadataCloseError(t *testing.T)
{
require.EqualError(t, err, "error on close")
require.Nil(t, createdView)
}
+
+func (t *ViewTestSuite) TestIdentifierReturnsDefensiveCopy() {
Review Comment:
tiny thing while we're here — this suite method landed after the standalone
`TestCreateViewReturnsMetadataCloseError` func. Testify wires it up fine either
way, but it'd read cleaner grouped with the other `*ViewTestSuite` methods
above.
--
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]
