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

scripts: dev script to start postgres before tests #3344

Merged
merged 7 commits into from Oct 27, 2019
Merged

Conversation

mniewrzal
Copy link
Contributor

@mniewrzal mniewrzal commented Oct 23, 2019

What: Script for developers to run integration tests easier after removing storj-sim sqlite support. This script wraps main script and adds postgres (docker) server start, waits for the server to be available and starts integration tests.

Why: Now all developers need to have postgres configured or use docker image. This should our lives a little bit.

Please describe the tests:

  • Test 1: none

Please describe the performance impact: none

Code Review Checklist (to be filled out by reviewer)

  • Does the PR describe what changes are being made?
  • Does the PR describe why the changes are being made?
  • Does the code follow our style guide?
  • Does the code follow our testing guide?
  • Is the PR appropriately sized? (If it could be broken into smaller PRs it should be)
  • Does the new code have enough tests? (every PR should have tests or justification otherwise. Bug-fix PRs especially)
  • Does the new code have enough documentation that answers "how do I use it?" and "what does it do?"? (both source documentation and higher level, diagrams?)
  • Does any documentation need updating?
  • Do the database access patterns make sense?

@mniewrzal mniewrzal added Request Code Review Code review requested Reviewer Can Merge If all checks have passed, non-owner can merge PR labels Oct 23, 2019
@mniewrzal mniewrzal requested a review from a team October 23, 2019 08:50
@cla-bot cla-bot bot added the cla-signed label Oct 23, 2019
@ghost ghost requested review from crawter and wthorp and removed request for a team October 23, 2019 08:50
scripts/test-sim-dev.sh Outdated Show resolved Hide resolved
@ifraixedes ifraixedes self-requested a review October 23, 2019 14:34
scripts/test-sim-dev.sh Outdated Show resolved Hide resolved
scripts/test-sim-dev.sh Outdated Show resolved Hide resolved
scripts/test-sim-dev.sh Outdated Show resolved Hide resolved
egonelbre
egonelbre previously approved these changes Oct 24, 2019
SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"

CONTAINER_NAME=storj_sim_postgres
docker run -d --rm -p 5433:5432 --name $CONTAINER_NAME postgres:9.6
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be really nice to be able to collect logs from this postgres somewhere, either live or before the container is destroyed. maybe the best way would be to do docker logs $CONTAINER_NAME > $tmpfile in the cleanup function? or perhaps starting up something like tail -f $(docker inspect --format '{{.LogPath}}' $CONTAINER_NAME) > $tmpfile & here?

And then we could add -c log_min_duration_statement=0 to this invocation, and get nice logs about every query that goes through, for those times when we care.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I will try to combine it somehow :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

live logging done

@mniewrzal mniewrzal merged commit da2eaa7 into master Oct 27, 2019
@mniewrzal mniewrzal deleted the mn/wrapper-script branch October 27, 2019 19:02
littleskunk pushed a commit that referenced this pull request Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed Request Code Review Code review requested Reviewer Can Merge If all checks have passed, non-owner can merge PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants