Skip to content
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

feature: return multierr on Close call #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

bbrodriges
Copy link
Contributor

Bumped minimal Go version to 1.20 to leverage new language feature: errors.Join. It allows to return all connections errors occurred while closing cluster.

@bbrodriges bbrodriges requested review from sidh and slon June 14, 2023 14:32
@@ -28,7 +28,6 @@ func TestClusterDefaults(t *testing.T) {
f := newFixture(t, 1)
c, err := NewCluster(f.ClusterNodes(), f.PrimaryChecker)
require.NoError(t, err)
defer func() { require.NoError(t, c.Close()) }()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Close is removed here and in other tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been removed due to pointlessness. The only reason it has been called is due to expectation in test suite generator. AFAIK hasql does not oblige user to call Close at any point.

As expectation has been removed so were all Close calls in tests. Close method has its own test function, so I believe all test cases relevant to cluster teardown must be placed there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants