Skip to content

feat(coderd/database/dbtestutil): add ability to dump database on failure #9704

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 6 commits into from
Sep 18, 2023

Conversation

johnstcn
Copy link
Member

@johnstcn johnstcn commented Sep 15, 2023

72dff7f included a provision to dump the contents of the database on failure, which actually seems really useful in a generic context.

  • Extracted this to a more general function exposed in dbtestutil.
  • This allows you to easily load data from a unit test and inspect the state at the time of test failure.
  • I chose column insert format as this is the most portable and explicit, but COPY FROM STDIN is another option.
  • I've chosen to not emit the dump to t.Log but this is also an option if running in CI. We just need to make sure to not leak any secrets.

@johnstcn johnstcn self-assigned this Sep 15, 2023
@johnstcn johnstcn force-pushed the cj/dbtestutil-dump-on-failure branch from d06e9c1 to 1c4bf16 Compare September 15, 2023 18:27
@johnstcn johnstcn marked this pull request as ready for review September 15, 2023 18:33
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Nice work!

I would love for these to be uploaded as artifacts in CI, but if we're cautious about leaking secrets that might be an issue.

return
}
snakeCaseName := regexp.MustCompile("[^a-zA-Z0-9-_]+").ReplaceAllString(t.Name(), "_")
outPath := filepath.Join(cwd, snakeCaseName+".test.sql")
Copy link
Member

Choose a reason for hiding this comment

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

How about adding a random suffix to make sure we don't overwrite previous dumps?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could add YYYYmmddHHMMSS so you know at a glance which one is the most recent?

Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

nit: you can also consider adding a short guide on how to import the dump into a local Coder deployment

@johnstcn johnstcn merged commit 1df7589 into main Sep 18, 2023
@johnstcn johnstcn deleted the cj/dbtestutil-dump-on-failure branch September 18, 2023 10:50
@github-actions github-actions bot locked and limited conversation to collaborators Sep 18, 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.

3 participants