Skip to content

chore: improve postgres test time by removing cleanup #7522

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

Merged
merged 1 commit into from
May 14, 2023
Merged

Conversation

kylecarbs
Copy link
Member

We don't need to delete databases on cleanup... and we don't need to always run without a cache either!

We don't need to delete databases on cleanup... and we don't need to
always run without a cache either!
@kylecarbs kylecarbs requested a review from coadler May 14, 2023 17:18
@kylecarbs kylecarbs self-assigned this May 14, 2023
-coverpkg=./... \
-count=1 -race -failfast
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove? I think it forces go test to not skip cached tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Go test caching is pretty effective in reducing only duplicates, we use it for the other jobs

Comment on lines +49 to +52
return "postgres://postgres:postgres@127.0.0.1:5432/" + dbName + "?sslmode=disable", func() {
// We don't need to clean anything up here... it's just a database in a container,
// so cleaning up the container will clean up the database.
}, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

is this func only used in ci? would be nice if this only applied in ci so we don't disable cleanup in dogfood.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nah, it's used outside too. We just don't need to clean this because it isn't actual waste

@kylecarbs kylecarbs merged commit 26490ae into main May 14, 2023
@kylecarbs kylecarbs deleted the psqltime branch May 14, 2023 20:32
@github-actions github-actions bot locked and limited conversation to collaborators May 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants