-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SNOW-540086: cover critical areas test code coverage for logger, dsn and connection #866
Conversation
3168c62
to
ec02780
Compare
ec02780
to
d37b153
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
if err = authenticateWithConfig(sc); err != nil { | ||
t.Error(err) | ||
} | ||
tx, _ := sc.Begin() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we close a transaction here?
if err != nil { | ||
t.Error(err) | ||
} | ||
sc, err := buildSnowflakeConn(ctx, *config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we call Close
at the end?
if err != nil { | ||
t.Error(err) | ||
} | ||
sc, err := buildSnowflakeConn(ctx, *config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we call Close at the end?
Description
Increase log.go coverage to 82.7% and cover missing critical areas in dsn.go and connection.go.
Checklist
make fmt
to fix inconsistent formatsmake lint
to get lint errors and fix all of them