-
Notifications
You must be signed in to change notification settings - Fork 12
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
implement testsuite and test-containers for repository integration test #332
Conversation
|
||
func (ns *NodeTestSuite) SetupSuite() { |
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.
Do we want to run many test suites? Isn't it better to have a single db instance and maybe, if we really need to do it, create different databases?
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.
Thanks. I thought creating multiple db containers would help to run the tests in parallel. But the idea of creating multiple db in a single container sounds good.
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.
Creating db instances in Postgres is very fast, definitely quicker than instantiating a container.
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.
updated with separate DB instead of separate container
21a0821
to
9e175d5
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.
It looks good - I just had one type suggestion.
test/database/postgres.go
Outdated
Password string | ||
DBName string | ||
Host string | ||
Port string |
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.
What do you think about making this an int
instead of a string, for a little more type-safety?
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.
updated
bbaf8f4
to
4e6a31d
Compare
closes #82