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

spanner: third party dependency exception for github.com/stretchr/testify #9378

Closed
noahdietz opened this issue Feb 6, 2024 · 4 comments · Fixed by #10087
Closed
Assignees
Labels
api: spanner Issues related to the Spanner API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: process A process-related concern. May include testing, release, or the like.

Comments

@noahdietz
Copy link
Contributor

Module: spanner
Usage(s): spanner/integration_test.go

For the time being, it will be exempt from the third party dep check, but please do one of the following:

A. Remove the dependency by switching to a package own directly by Google, handwriting necessary functionality, or using stdlib
B. Justify the exception and ack the risks, maintaining the exception indefinitely

@noahdietz noahdietz added the type: process A process-related concern. May include testing, release, or the like. label Feb 6, 2024
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Feb 6, 2024
@rahul2393 rahul2393 added the priority: p3 Desirable enhancement or fix. May not be included in next release. label Feb 12, 2024
@rahul2393
Copy link
Contributor

Used in tests only.

@egonelbre
Copy link
Contributor

egonelbre commented Apr 10, 2024

I don't have any concerns with testify package personally, but in this case the dependency removal is trivial. i.e.

require.NoError(t, err)

// replace
require.NoError(t, err)

// with
if err != nil {
    t.Errorf("Apply returns error %v, want nil", err)
}

EDIT: just realized that it's also used via mockery.

egonelbre added a commit to egonelbre/google-cloud-go that referenced this issue Apr 12, 2024
rahul2393 pushed a commit that referenced this issue May 2, 2024
* chore(spanner): remove the need for github.com/stretchr/testify

Updates #9378

* go mod tidy an additional file

* add missing empty line
@noahdietz
Copy link
Contributor Author

Awesome, thank you @egonelbre

@noahdietz noahdietz reopened this May 2, 2024
@noahdietz
Copy link
Contributor Author

Need to remove the exemption now, oops

gcf-merge-on-green bot pushed a commit that referenced this issue May 2, 2024
Since `testify` was removed from spanner in #9760 we can remove the exemption for it, so that it won't be added back.

Fixes #9378
jschaf pushed a commit to jschaf/spanner-go that referenced this issue Oct 10, 2024
* chore(spanner): remove the need for github.com/stretchr/testify

Updates googleapis/google-cloud-go#9378

* go mod tidy an additional file

* add missing empty line
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: process A process-related concern. May include testing, release, or the like.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants