-
Notifications
You must be signed in to change notification settings - Fork 153
Infra - wire up database for test #2119
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
Infra - wire up database for test #2119
Conversation
5a2721b
to
ee95395
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.
Thank you! Looks great, left a few notes.
8ada40f
to
9eac5a4
Compare
…support docker containers in githubs CI)
b33b844
to
998c309
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.
I'm not such a big fan of the Makefile
, especially since when running the containers with -d
, it becomes too easy to let the container continue running in the background, which can slow down the system until restart. It also won't easily work on Windows. But I don't mind keeping it.
What we should add though is some documentation of this, because if someone just runs cargo test
, the failure won't tell them much.
Could you please add a ## How to test
section to collector/README.md
that explains the need for the Postgres DB in 1-2 sentences and shows the make test
command? Thank you.
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.
Thank you!
I've added a Makefile too so we can spin it up and down locally.